plotting functions for d2 and scattering cross section - #93
Merged
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new plot_d2 currently drops the DataArray’s k coordinate (misaligning the x-axis/best-k marker), and the updated tests use an inconsistent FWHM→σ conversion constant.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds a new visualization helper to plot Moteki & Kondo statistical distance (d^2(k)) and updates the existing NDM/plotting tests to apply width-to-sigma conversions more consistently.
Changes:
- Added
plot_d2to visualize (d^2(k)) with optional log scaling, threshold line, and best-(k) highlighting. - Added
test_plot_d2(mpl image comparison) and importedplot_d2into the plotting test suite. - Updated NDM and plotting tests to convert width-like inputs to (\sigma) before constructing
MLEConfig.
File summaries
| File | Description |
|---|---|
pysp2/util/normalized_derivative_method.py |
Adds plot_d2 plotting function for Moteki & Kondo (d^2(k)). |
tests/test_vis.py |
Imports plot_d2, adds image-based test for it, and updates width conversion constants in plotting-related tests. |
tests/test_ndm.py |
Updates width conversion constants in Moteki & Kondo NDM test configuration. |
Review details
Suppressed comments (2)
tests/test_vis.py:119
- Same conversion issue as above: 2.335 is not the standard Gaussian FWHM→σ factor used elsewhere in this repo (2.35482). Keeping a single factor avoids subtle mismatches across plotting tests.
sigma_bar= (18.5/2.335)*0.4, # example; use your measured average width
delta_sigma=(1.2/2.335)*0.4, # example; use your measured width std dev
tests/test_vis.py:183
- Same conversion issue as the other plotting tests: 2.335 should be 2.35482 (Gaussian FWHM→σ), consistent with other modules (e.g., plot_wave / leo_fit).
sigma_bar= (18.5/2.335)*0.4, # example; use your measured average width
delta_sigma=(1.2/2.335)*0.4, # example; use your measured width std dev
- Files reviewed: 3/5 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
rcjackson
approved these changes
Sep 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new visualization function for statistical distance analysis and updates test cases for plotting functions to use more accurate parameter conversions. The main highlights are the addition of the
plot_d2function to visualize Moteki & Kondo statistical distance, and the corresponding test to ensure its correctness. Additionally, all test cases now use FWTM-to-sigma conversions for relevant parameters, improving consistency and accuracy.New plotting functionality:
plot_d2function topysp2/util/normalized_derivative_method.pyfor visualizing Moteki & Kondo statistical distanceTesting improvements:
test_plot_d2intests/test_vis.pyto validate the newplot_d2function using pytest's image comparison, ensuring visual correctness of the plot.plot_d2intests/test_vis.pyto enable testing of the new plotting function.Parameter conversion consistency:
test_ndm_moteki_kondo,test_plot_incident_irradiance,test_plot_scattering_cross_section, andtest_plot_d2) to convert FWTM values to sigma by dividing by 2.335, ensuring parameter consistency with the model's requirements. [1] [2] [3] [4]