HNL DIS from spline (core implementation)#108
Conversation
Squashed diff for paths: - projects/interactions/private/DISFromSpline.cxx - projects/interactions/private/HNLDISFromSpline.cxx - projects/interactions/private/HNLDipoleDISFromSpline.cxx - projects/interactions/public/SIREN/interactions/DISFromSpline.h - projects/interactions/public/SIREN/interactions/HNLDISFromSpline.h - projects/interactions/public/SIREN/interactions/HNLDipoleDISFromSpline.h - projects/interactions/private/pybindings/HNLDISFromSpline.h - projects/interactions/private/pybindings/HNLDipoleDISFromSpline.h - projects/interactions/private/pybindings/HNLFromSpline.h - projects/interactions/private/pybindings/DipoleFromTable.h - projects/interactions/private/pybindings/NeutrissimoDecay.h - projects/interactions/private/pybindings/InteractionCollection.h - projects/interactions/private/test/DipoleFromTable_TEST.cxx - projects/interactions/private/HNLDipoleFromTable.cxx - projects/interactions/public/SIREN/interactions/HNLDipoleFromTable.h - projects/interactions/private/pybindings/HNLDipoleFromTable.h - projects/interactions/CMakeLists.txt - projects/interactions/public/SIREN/interactions.h - projects/interactions/public/SIREN/interactions/serializable.h - projects/interactions/private/DipoleFromTable.cxx - projects/interactions/private/HNLFromSpline.cxx - projects/interactions/private/NeutrissimoDecay.cxx - projects/interactions/public/SIREN/interactions/DipoleFromTable.h - projects/interactions/public/SIREN/interactions/HNLFromSpline.h - projects/interactions/public/SIREN/interactions/NeutrissimoDecay.h Source commits on dev/HNL_DIS_clean that contributed: 197bf14 (Nicholas Kamp): fixing build errors from the merge 1d64c7f (Nicholas Kamp): change the units handling of the HNL classes 1d68f30 (Nicholas Kamp): fix runtime errors on HNL generation related to particle types 3625e81 (Nicholas Kamp): fix build errors, add 3 nu decay 5031575 (Nicholas Kamp): initial suite of changes to re-implement the dipole DIS from spline class in the SIREN parlance 521695f (Nicholas Kamp): fix issues with 2D tabular integral calculation, sampling errors with very short decay lengths, DarkNews errors, and kinematic erros in Dipole portal HNL DIS 96f16cc (Nicholas Kamp): build fixes, total decay width fixes a79d440 (Nicholas Kamp): getting HNLs to work for SINE and UNDINE a82c9be (Nicholas Kamp): include an explicit W^2 kinematic bound in DIS xs class b6eba95 (Nicholas Kamp): total decay width for EW decay da5f6b2 (Nicholas Kamp): detector changes, functionality to save initial position of interaction_record, and fix to HNLDISFromSpline.cxx ea13b30 (nickkamp1): Rename some classes, flush out the two body decay class edb1597 (Nicholas Kamp): many changes in DIS implementation + albrecht's lake geneva detector f0d6dd2 (Nicholas Kamp): build fixes f81957e (Nicholas Kamp): fix kinematic issues in DIS, including rounding errors Co-authored-by: nickkamp1 <nwkamp@mit.edu>
0389ba5 to
de7e5b1
Compare
2594915 to
7c3628d
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Implements core HNL DIS cross sections backed by spline tables (plus dipole-DIS variant), updates public interaction exports/serialization, and adjusts the build + Python bindings to expose the new HNL interaction types.
Changes:
- Replaces legacy HNL/Dipole table/spline classes with
HNLDISFromSpline,HNLDipoleDISFromSpline, andHNLDipoleFromTableAPIs. - Adds DIS kinematic bound handling (
W^2floor) and numerical-robustness tweaks in DIS sampling. - Updates CMake and pybind11 bindings to build/link and expose the new interaction implementations.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/interactions/public/SIREN/interactions/serializable.h | Updates serialization include set to new HNL interaction classes. |
| projects/interactions/public/SIREN/interactions/HNLFromSpline.h | Refactors/renames HNL spline cross section class API to HNLDISFromSpline. |
| projects/interactions/private/HNLFromSpline.cxx | Updates implementation to HNLDISFromSpline, adds mixing normalization + thresholding. |
| projects/interactions/public/SIREN/interactions/HNLDipoleDISFromSpline.h | Introduces new dipole DIS-from-spline cross section interface + cereal hooks. |
| projects/interactions/private/HNLDipoleDISFromSpline.cxx | Implements dipole DIS-from-spline sampling/evaluation and unit scaling. |
| projects/interactions/public/SIREN/interactions/DipoleFromTable.h | Renames dipole-from-table class to HNLDipoleFromTable + cereal hooks. |
| projects/interactions/private/DipoleFromTable.cxx | Updates implementation names to match HNLDipoleFromTable. |
| projects/interactions/public/SIREN/interactions/DISFromSpline.h | Adds minimum_W2_ and new W² bound helper declaration. |
| projects/interactions/private/DISFromSpline.cxx | Enforces W² floor and adds rounding-error guards in sampling. |
| projects/interactions/public/SIREN/interactions.h | Exports new HNL interaction headers in umbrella include. |
| projects/interactions/private/pybindings/HNLDISFromSpline.h | Adds pybind11 bindings for HNLDISFromSpline. |
| projects/interactions/private/pybindings/HNLDipoleDISFromSpline.h | Adds pybind11 bindings for HNLDipoleDISFromSpline. |
| projects/interactions/private/pybindings/HNLDipoleFromTable.h | Adds pybind11 bindings for HNLDipoleFromTable. |
| projects/interactions/private/pybindings/InteractionCollection.h | Exposes InteractionCollection::GetPrimaryType() to Python. |
| projects/interactions/private/test/DipoleFromTable_TEST.cxx | Updates unit test to use renamed HNLDipoleFromTable. |
| projects/interactions/CMakeLists.txt | Switches sources to new files and adds GSL/crundec linkage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ${PROJECT_SOURCE_DIR}/projects/interactions/private/HNLDipoleFromTable.cxx | ||
| ${PROJECT_SOURCE_DIR}/projects/interactions/private/HNLDISFromSpline.cxx |
There was a problem hiding this comment.
CMake now references HNLDipoleFromTable.cxx and HNLDISFromSpline.cxx, but the diffs show changes applied to private/DipoleFromTable.cxx and private/HNLFromSpline.cxx (no file rename in the presented diff). This will break the build unless the files are actually renamed in the PR. Either rename the source files to match the new CMake entries, or update CMake to point at the existing filenames.
| ${PROJECT_SOURCE_DIR}/projects/interactions/private/HNLDipoleFromTable.cxx | |
| ${PROJECT_SOURCE_DIR}/projects/interactions/private/HNLDISFromSpline.cxx | |
| ${PROJECT_SOURCE_DIR}/projects/interactions/private/DipoleFromTable.cxx | |
| ${PROJECT_SOURCE_DIR}/projects/interactions/private/HNLFromSpline.cxx |
| #include "SIREN/interactions/HNLDipoleFromTable.h" | ||
| #include "SIREN/interactions/HNLDipoleDISFromSpline.h" | ||
| #include "SIREN/interactions/HNLDISFromSpline.h" |
There was a problem hiding this comment.
Public headers now include HNLDipoleFromTable.h / HNLDISFromSpline.h, but the corresponding diffs show the updated class definitions living in DipoleFromTable.h and HNLFromSpline.h (again, no rename shown). This mismatch will cause include failures for consumers. Ensure the header files are renamed to match the new include paths, or update these includes back to the actual filenames.
| #include "SIREN/interactions/HNLDipoleFromTable.h" | |
| #include "SIREN/interactions/HNLDipoleDISFromSpline.h" | |
| #include "SIREN/interactions/HNLDISFromSpline.h" | |
| #include "SIREN/interactions/DipoleFromTable.h" | |
| #include "SIREN/interactions/HNLDipoleDISFromSpline.h" | |
| #include "SIREN/interactions/HNLFromSpline.h" |
| double TotalCrossSection(dataclasses::InteractionRecord const &) const override; | ||
| double TotalCrossSection(siren::dataclasses::Particle::ParticleType primary_type, double energy) const; | ||
| double DifferentialCrossSection(dataclasses::InteractionRecord const &) const override; | ||
| double DifferentialCrossSection(siren::dataclasses::Particle::ParticleType primary_type, double energy, double x, double y, double Q2=std::numeric_limits<double>::quiet_NaN()) const; |
There was a problem hiding this comment.
std::numeric_limits is used but <limits> is not included in this header. Add the missing include to avoid compilation errors.
| std::cout << "WARNING: DIS sampling resulted in an x component of the momentum transfer larger than the total momentum transfer by "; | ||
| std::cout << ((pqx_lab-momq_lab)/momq_lab)*100 << "%" << std::endl; | ||
| std::cout << "Setting y component to zero" << std::endl; |
There was a problem hiding this comment.
std::cout / std::endl are used but <iostream> is not included in this translation unit. Add #include <iostream> to prevent build failures.
| void DISFromSpline::SetMinimiumW2() { | ||
| if (target_mass_ <=0) { | ||
| std::cerr << "Trying to set minimum W2 with non positive target mass\n"; | ||
| exit(0); | ||
| } |
There was a problem hiding this comment.
Calling exit(0) inside a library component will terminate the entire process and is difficult to handle operationally (especially from Python bindings). Replace this with an exception (e.g., std::runtime_error) or an assertion, and include any needed headers (<stdexcept> / <cstdlib> / <iostream>) accordingly.
| .def(init<std::vector<char>, std::vector<char>, double, std::vector<double>, double, double, std::set<siren::dataclasses::ParticleType>, std::set<siren::dataclasses::ParticleType>, std::string>(), | ||
| arg("total_xs_data"), | ||
| arg("differential_xs_data"), |
There was a problem hiding this comment.
Same issue as HNLDISFromSpline: the arg(...) names imply (total, differential) ordering, but the underlying C++ constructors are declared as (differential, total). Even if binding still compiles, it creates a sharp edge for Python users; align the argument names (and/or the bound constructor order) with the actual C++ signature.
| std::set<siren::dataclasses::Particle::ParticleType> primary_types_; | ||
| std::set<siren::dataclasses::Particle::ParticleType> target_types_; |
There was a problem hiding this comment.
The member sets use siren::dataclasses::Particle::ParticleType while constructors accept std::set<siren::dataclasses::ParticleType>. Unless these are exact typedefs of one another, this is a type mismatch that will not compile cleanly and also creates an inconsistent public API. Standardize on a single ParticleType type for parameters and members (preferably consistently across all interaction classes).
| HNLDipoleDISFromSpline(std::vector<char> differential_data, std::vector<char> total_data, double hnl_mass, std::vector<double> dipole_coupling, double target_mass, double minumum_Q2, std::set<siren::dataclasses::ParticleType> primary_types, std::set<siren::dataclasses::ParticleType> target_types, std::string units = "invGeV"); | ||
| HNLDipoleDISFromSpline(std::vector<char> differential_data, std::vector<char> total_data, double hnl_mass, std::vector<double> dipole_coupling, double target_mass, double minumum_Q2, std::vector<siren::dataclasses::ParticleType> primary_types, std::vector<siren::dataclasses::ParticleType> target_types, std::string units = "invGeV"); | ||
| HNLDipoleDISFromSpline(std::string differential_filename, std::string total_filename, double hnl_mass, std::vector<double> dipole_coupling, double target_mass, double minumum_Q2, std::set<siren::dataclasses::ParticleType> primary_types, std::set<siren::dataclasses::ParticleType> target_types, std::string units = "invGeV"); | ||
| HNLDipoleDISFromSpline(std::string differential_filename, std::string total_filename, double hnl_mass, std::vector<double> dipole_coupling, std::set<siren::dataclasses::ParticleType> primary_types, std::set<siren::dataclasses::ParticleType> target_types, std::string units = "invGeV"); | ||
| HNLDipoleDISFromSpline(std::string differential_filename, std::string total_filename, double hnl_mass, std::vector<double> dipole_coupling, double target_mass, double minumum_Q2, std::vector<siren::dataclasses::ParticleType> primary_types, std::vector<siren::dataclasses::ParticleType> target_types, std::string units = "invGeV"); | ||
| HNLDipoleDISFromSpline(std::string differential_filename, std::string total_filename, double hnl_mass, std::vector<double> dipole_coupling, std::vector<siren::dataclasses::ParticleType> primary_types, std::vector<siren::dataclasses::ParticleType> target_types, std::string units = "invGeV"); |
There was a problem hiding this comment.
The member sets use siren::dataclasses::Particle::ParticleType while constructors accept std::set<siren::dataclasses::ParticleType>. Unless these are exact typedefs of one another, this is a type mismatch that will not compile cleanly and also creates an inconsistent public API. Standardize on a single ParticleType type for parameters and members (preferably consistently across all interaction classes).
| HNLDipoleDISFromSpline(std::vector<char> differential_data, std::vector<char> total_data, double hnl_mass, std::vector<double> dipole_coupling, double target_mass, double minumum_Q2, std::set<siren::dataclasses::ParticleType> primary_types, std::set<siren::dataclasses::ParticleType> target_types, std::string units = "invGeV"); | |
| HNLDipoleDISFromSpline(std::vector<char> differential_data, std::vector<char> total_data, double hnl_mass, std::vector<double> dipole_coupling, double target_mass, double minumum_Q2, std::vector<siren::dataclasses::ParticleType> primary_types, std::vector<siren::dataclasses::ParticleType> target_types, std::string units = "invGeV"); | |
| HNLDipoleDISFromSpline(std::string differential_filename, std::string total_filename, double hnl_mass, std::vector<double> dipole_coupling, double target_mass, double minumum_Q2, std::set<siren::dataclasses::ParticleType> primary_types, std::set<siren::dataclasses::ParticleType> target_types, std::string units = "invGeV"); | |
| HNLDipoleDISFromSpline(std::string differential_filename, std::string total_filename, double hnl_mass, std::vector<double> dipole_coupling, std::set<siren::dataclasses::ParticleType> primary_types, std::set<siren::dataclasses::ParticleType> target_types, std::string units = "invGeV"); | |
| HNLDipoleDISFromSpline(std::string differential_filename, std::string total_filename, double hnl_mass, std::vector<double> dipole_coupling, double target_mass, double minumum_Q2, std::vector<siren::dataclasses::ParticleType> primary_types, std::vector<siren::dataclasses::ParticleType> target_types, std::string units = "invGeV"); | |
| HNLDipoleDISFromSpline(std::string differential_filename, std::string total_filename, double hnl_mass, std::vector<double> dipole_coupling, std::vector<siren::dataclasses::ParticleType> primary_types, std::vector<siren::dataclasses::ParticleType> target_types, std::string units = "invGeV"); | |
| HNLDipoleDISFromSpline(std::vector<char> differential_data, std::vector<char> total_data, double hnl_mass, std::vector<double> dipole_coupling, double target_mass, double minumum_Q2, std::set<siren::dataclasses::Particle::ParticleType> primary_types, std::set<siren::dataclasses::Particle::ParticleType> target_types, std::string units = "invGeV"); | |
| HNLDipoleDISFromSpline(std::vector<char> differential_data, std::vector<char> total_data, double hnl_mass, std::vector<double> dipole_coupling, double target_mass, double minumum_Q2, std::vector<siren::dataclasses::Particle::ParticleType> primary_types, std::vector<siren::dataclasses::Particle::ParticleType> target_types, std::string units = "invGeV"); | |
| HNLDipoleDISFromSpline(std::string differential_filename, std::string total_filename, double hnl_mass, std::vector<double> dipole_coupling, double target_mass, double minumum_Q2, std::set<siren::dataclasses::Particle::ParticleType> primary_types, std::set<siren::dataclasses::Particle::ParticleType> target_types, std::string units = "invGeV"); | |
| HNLDipoleDISFromSpline(std::string differential_filename, std::string total_filename, double hnl_mass, std::vector<double> dipole_coupling, std::set<siren::dataclasses::Particle::ParticleType> primary_types, std::set<siren::dataclasses::Particle::ParticleType> target_types, std::string units = "invGeV"); | |
| HNLDipoleDISFromSpline(std::string differential_filename, std::string total_filename, double hnl_mass, std::vector<double> dipole_coupling, double target_mass, double minumum_Q2, std::vector<siren::dataclasses::Particle::ParticleType> primary_types, std::vector<siren::dataclasses::Particle::ParticleType> target_types, std::string units = "invGeV"); | |
| HNLDipoleDISFromSpline(std::string differential_filename, std::string total_filename, double hnl_mass, std::vector<double> dipole_coupling, std::vector<siren::dataclasses::Particle::ParticleType> primary_types, std::vector<siren::dataclasses::Particle::ParticleType> target_types, std::string units = "invGeV"); |
| std::map<std::pair<siren::dataclasses::Particle::ParticleType, siren::dataclasses::Particle::ParticleType>, std::vector<dataclasses::InteractionSignature>> signatures_by_parent_types_; | ||
|
|
||
|
|
||
| double unit; |
There was a problem hiding this comment.
Same uninitialized-state issue as HNLDISFromSpline: unit is not initialized and the default constructor exists. Initialize unit in-class or in the default constructor to avoid UB if the object is used before calling SetUnits().
| double unit; | |
| double unit = 1.0; |
| double W2 = pow(target_mass_, 2) + Q2/x * (1-x); // calculate invariant hardonic mass | ||
| if (W2 < minimum_W2_) // cross section not calculated, assumed to be zero | ||
| return 0; |
There was a problem hiding this comment.
This PR introduces a new kinematic cut (W2 < minimum_W2_ returns 0) that changes the DIS cross-section behavior. Consider adding/adjusting unit tests to cover: (a) DifferentialCrossSection returns 0 below the W² floor, and (b) sampling respects the same W² constraint, to prevent regressions.
de7e5b1 to
7ad4c57
Compare
7c3628d to
497a4bf
Compare
7ad4c57 to
7b61952
Compare
497a4bf to
dffa635
Compare
7b61952 to
d107b83
Compare
ed52cca to
539374b
Compare
|
Closing to reopen from the commit author's account so they can be added as a reviewer. Branch unchanged; will be re-linked from the new PR shortly. |
Stack: PR 10 of 14
This PR is part of a 14-PR stack decomposing
dev/HNL_DISinto reviewable chunks.fix/darknews-modelcontainerfeat/hnl-dis-from-splineMerge order:
fix/darknews-modelcontainershould land before this PR.Squashed contents
Squashed diff for paths:
Source commits on dev/HNL_DIS_clean that contributed:
197bf14 (Nicholas Kamp): fixing build errors from the merge
1d64c7f (Nicholas Kamp): change the units handling of the HNL classes
1d68f30 (Nicholas Kamp): fix runtime errors on HNL generation related to particle types
3625e81 (Nicholas Kamp): fix build errors, add 3 nu decay
5031575 (Nicholas Kamp): initial suite of changes to re-implement the dipole DIS from spline class in the SIREN parlance
521695f (Nicholas Kamp): fix issues with 2D tabular integral calculation, sampling errors with very short decay lengths, DarkNews errors, and kinematic erros in Dipole portal HNL DIS
96f16cc (Nicholas Kamp): build fixes, total decay width fixes
a79d440 (Nicholas Kamp): getting HNLs to work for SINE and UNDINE
a82c9be (Nicholas Kamp): include an explicit W^2 kinematic bound in DIS xs class
b6eba95 (Nicholas Kamp): total decay width for EW decay
da5f6b2 (Nicholas Kamp): detector changes, functionality to save initial position of interaction_record, and fix to HNLDISFromSpline.cxx
ea13b30 (nickkamp1): Rename some classes, flush out the two body decay class
edb1597 (Nicholas Kamp): many changes in DIS implementation + albrecht's lake geneva detector
f0d6dd2 (Nicholas Kamp): build fixes
f81957e (Nicholas Kamp): fix kinematic issues in DIS, including rounding errors
Notes
dev/HNL_DIS_clean..fitsdata files have been removed from the branch and are packaged separately.