Skip to content

fixes to d2 - #94

Merged
rcjackson merged 3 commits into
ARM-DOE:mainfrom
jtgasparik:troubleshoot_d2
Sep 10, 2026
Merged

fixes to d2 #94
rcjackson merged 3 commits into
ARM-DOE:mainfrom
jtgasparik:troubleshoot_d2

Conversation

@jtgasparik

Copy link
Copy Markdown
Collaborator

This pull request updates the baseline handling and parameterization for the Moteki-Kondo normalized derivative method, improves consistency with related methods, and refines test tolerances and parameters for better accuracy and reproducibility. The most important changes are grouped below:

Core logic improvements

  • Introduced a new helper function _baseline_to_zero to shift event signals so their minimum is zero, ensuring baseline handling matches the central_difference method. This function is now used throughout the Moteki-Kondo computation functions (_tau_hat_for_one_event, compute_d2_moteki_kondo, and compute_sigma_moteki_kondo). [1] [2] [3] [4]

Parameter and threshold adjustments

  • Lowered the default d2_threshold parameter in compute_sigma_moteki_kondo from 80000.0 to 20.0, likely to better reflect the expected range for the updated baseline handling.

Test parameter updates

  • Updated the A1, A2, and A3 coefficients used in test cases to new values optimized for the current dataset, replacing the previous scaling approach. This affects tests in both test_ndm.py and test_vis.py. [1] [2] [3] [4]

Test tolerance refinements

  • Relaxed tolerances in assertions within tests to accommodate the new baseline handling and parameter changes, ensuring tests remain robust to expected variation. [1] [2] [3] [4]

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

It introduces significant behavioral/API default changes (baseline shifting and d2_threshold) that need clearer configurability/documentation, and there are test-comment accuracy/provenance issues to resolve.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates Moteki–Kondo normalized derivative method computations to use consistent baseline handling (shifting each event’s scattering signal so its minimum is zero), adjusts sigma-estimation acceptance threshold defaults, and updates test parameters/tolerances to match the new behavior.

Changes:

  • Added _baseline_to_zero and applied it when extracting per-event S for Moteki–Kondo tau/d²/sigma computations.
  • Changed compute_sigma_moteki_kondo default d2_threshold from 80000.0 to 20.0.
  • Updated test coefficients (A1, A2, A3) and relaxed several test tolerances; refined test comments.
File summaries
File Description
pysp2/util/normalized_derivative_method.py Adds baseline-shift helper, applies it in Moteki–Kondo routines, and changes default d2_threshold.
tests/test_ndm.py Updates Moteki–Kondo test parameters/tolerances and related commentary.
tests/test_vis.py Updates visualization tests’ Moteki–Kondo parameters and adds explanatory comments.
Review details
  • Files reviewed: 3/6 changed files
  • Comments generated: 5
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pysp2/util/normalized_derivative_method.py
Comment thread pysp2/util/normalized_derivative_method.py
Comment thread tests/test_ndm.py
Comment thread tests/test_ndm.py
Comment thread tests/test_vis.py
@rcjackson
rcjackson merged commit 0e721b3 into ARM-DOE:main Sep 10, 2026
20 checks passed
@jtgasparik
jtgasparik deleted the troubleshoot_d2 branch September 10, 2026 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants