Conversation
beykyle
changed the title
There was a problem hiding this comment.
Pull request overview
This PR merges recent scattering/xs functionality, including frame-angle conversion helpers, updated spin observables (adds spin-rotation parameter Q), and expands CI to run example notebooks as tests.
Changes:
- Add spin-rotation observable Q to elastic differential cross-section calculations and adjust S/T-matrix element extraction.
- Add
Reaction.to_lab_frame()/Reaction.to_cm_frame()helpers and clarify kinematics docs. - Update CI to test notebooks and add notebook-related requirements/docs; various notebook/example updates and import cleanups.
Reviewed changes
Copilot reviewed 25 out of 35 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/jitr/xs/elastic.py |
Adds Q to ElasticXS and computes Q in differential_elastic_xs; adjusts solver return handling. |
src/jitr/xs/quasielastic_pn.py |
Updates spherical harmonic usage and fixes scalar extraction from S-matrix returns. |
src/jitr/reactions/reaction.py |
Adds CM↔lab angle conversion helpers; adjusts absorption/gamma capture residual construction. |
src/jitr/utils/kinematics.py |
Clarifies lab-frame definition in docstrings. |
src/jitr/optical_potentials/wlh.py |
Tweaks sign convention for an absorptive parameter with explanatory comment. |
requirements.txt |
Adds pytest requirement. |
.github/workflows/python-package.yml |
Expands Python matrix and adds notebook execution tests. |
README.md |
Documents notebook requirements and how to run notebook tests. |
test/test_*.py |
Minor import/format cleanups in tests. |
examples/*.py and examples/notebooks/* |
Notebook updates/additions and example script import cleanups; notebooks intended for CI testing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def to_lab_frame(self, theta_cm: np.ndarray, Ecm: float, Q: float) -> np.ndarray: | ||
| """ | ||
| Convert angles from the center-of-mass frame to the laboratory frame | ||
| (target rest frame). | ||
| Args: | ||
| theta_cm (np.ndarray): Angles in the center-of-mass frame in degrees. | ||
| Ecm (float): Center-of-mass energy. | ||
| Q (float): Q-value of the reaction. | ||
| """ | ||
| return cm_to_lab_frame( | ||
| theta_cm, | ||
| self.projectile.m0, | ||
| self.target.m0, | ||
| self.product.m0, | ||
| self.residual.m0, | ||
| Ecm, | ||
| Q, | ||
| ) | ||
|
|
||
| def to_cm_frame(self, theta_lab: np.ndarray, Elab: float, Q: float) -> np.ndarray: | ||
| """ | ||
| Convert angles from the laboratory (target rest frame) frame to the | ||
| center-of-mass frame. | ||
| Args: | ||
| theta_lab (np.ndarray): Angles in the laboratory frame in degrees. | ||
| Elab (float): Laboratory energy. | ||
| Q (float): Q-value of the reaction. | ||
| """ | ||
| return lab_to_cm_frame( | ||
| theta_lab, | ||
| self.projectile.m0, | ||
| self.target.m0, | ||
| self.product.m0, | ||
| self.residual.m0, | ||
| Elab, | ||
| Q, | ||
| ) |
There was a problem hiding this comment.
Reaction.to_lab_frame() / to_cm_frame() unconditionally dereference self.product and self.residual. For processes where either is None (e.g., tot, non, x, and possibly inl depending on configuration), calling these helpers will raise an AttributeError. Consider either (1) validating that product and residual are present and raising a clear ValueError, or (2) moving these methods to only reaction types where they are always defined (e.g., 2-body reactions).
There was a problem hiding this comment.
@copilot open a new pull request to fix this, I like suggestion (1) this feedback
Co-authored-by: beykyle <22779182+beykyle@users.noreply.github.com>
Guard `to_lab_frame`/`to_cm_frame` against undefined product/residual
Uh oh!
There was an error while loading. Please reload this page.