Skip to content

Scalar (mediator) Dark Matter at CCM#92

Open
austinschneider wants to merge 27 commits into
mainfrom
sdm
Open

Scalar (mediator) Dark Matter at CCM#92
austinschneider wants to merge 27 commits into
mainfrom
sdm

Conversation

@austinschneider

Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings December 23, 2025 16:31

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.

Pull request overview

This PR adds support for Scalar (mediator) Dark Matter at the CCM detector. The changes include API refactoring to distinguish between total decay widths across all final states versus specific final states, infrastructure improvements to handle injection attempts and step lengths, and new distribution classes for sampling pre-generated kinematics data.

Key changes:

  • Refactored decay API to separate TotalDecayWidthAllFinalStates from TotalDecayWidth for better clarity
  • Added injection attempt tracking and step length propagation through the event generation pipeline
  • Introduced three new distribution classes for sampling from pre-generated data (position, energy, and full kinematics)
  • Enhanced error handling in Romberg integration and detector model intersection calculations
  • Fixed various bugs in direction handling and column depth calculations

Reviewed changes

Copilot reviewed 54 out of 59 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
vendor/photospline Updated subproject commit
vendor/cereal Updated subproject commit
resources/processes/SDMCrossSection/process.py New scalar dark matter cross section calculation implementation
resources/examples/example2/PaperPlots.ipynb Updated Python version reference
python/_util.py Added early return for missing detector resources
python/init.py Added success message on import
python/Weighter.py Removed validation check for primary physical distributions
projects/utilities/public/SIREN/utilities/Pybind11Trampoline.h Added new trampoline macros for reference return types
projects/utilities/public/SIREN/utilities/Integration.h Enhanced Romberg integration with better error handling
projects/interactions/public/SIREN/interactions/* Renamed decay methods across multiple headers for API clarity
projects/interactions/private/* Updated implementations to match new decay API
projects/injection/public/SIREN/injection/Weighter.tcc Updated to use new decay length methods
projects/injection/public/SIREN/injection/Injector.h Added injection attempts tracking
projects/injection/private/Injector.cxx Refactored event generation error handling and added injection attempt counter
projects/injection/private/WeightingUtils.cxx Updated target calculation and added zero probability check
projects/distributions/public/SIREN/distributions/primary/* Added three new distribution classes for sampling
projects/distributions/private/* Implementation files for new distributions and bug fixes
projects/detector/private/DetectorModel.cxx Fixed direction handling and enhanced target validation
projects/dataclasses/public/SIREN/dataclasses/* Added injection_step_length field to particle and record structures
cmake/parse_pyproject.py Fixed import error handling
CMakeLists.txt Added debug configuration and Python staging directory
Comments suppressed due to low confidence (7)

resources/processes/SDMCrossSection/process.py:1

  • Variable m_e is defined on line 37 but then redefined on line 61 with a different value (0.000511 vs 0.00051099892). This duplicate definition will cause the first definition to be overwritten, which may lead to inconsistent behavior.
    resources/processes/SDMCrossSection/process.py:1
  • Function name sm_masses conflicts with the dictionary defined on line 18. This shadowing makes the dictionary inaccessible after this function definition and could cause confusion.
    resources/processes/SDMCrossSection/process.py:1
  • Parameter m_nucleus is duplicated in the function signature. The parameter appears twice at positions 3 and 5, which is a syntax error in Python.
    resources/processes/SDMCrossSection/process.py:1
  • References to self (lines 156, 165) in a standalone function get_CM_factors. This function is not a method and has no self parameter, which will cause a NameError at runtime.
    resources/processes/SDMCrossSection/process.py:1
  • Attribute self.g_gamgam is used but not defined in the __init__ method. The class initializes self.g_phi_e_e instead. This will cause an AttributeError.
    python/init.py:1
  • The import of builtins on line 58 appears unused, and the print statement on line 59 is not appropriate for a production library as it produces output on every import.
    projects/utilities/public/SIREN/utilities/Integration.h:1
  • Returning ext when extErr is NaN could propagate invalid numerical results. Consider throwing an exception or logging an error instead of silently returning a potentially incorrect value.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread projects/injection/private/Injector.cxx Outdated
}
primary_record.Finalize(record);
SampleCrossSection(record);
injected_events += 1;

Copilot AI Dec 23, 2025

Copy link

Choose a reason for hiding this comment

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

Returning an empty InteractionTree on injection failure (line 305) but then incrementing injected_events on line 345. This creates inconsistency where failed injections are counted as successful events.

Suggested change
injected_events += 1;

Copilot uses AI. Check for mistakes.
SamplePrimaryPositionDistribution::SamplePrimaryPositionDistribution(std::vector<siren::math::Vector3D> positions, std::vector<double> weights)
: positions_(positions), weights_(weights) {
if (weights_.empty()) {
std::cout << "Caution: No weights are used in association with the position data. This assumes you're data is unbiased and it will be sampled uniformly!" << std::endl;

Copilot AI Dec 23, 2025

Copy link

Choose a reason for hiding this comment

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

Corrected spelling of 'you're' to 'your'.

Suggested change
std::cout << "Caution: No weights are used in association with the position data. This assumes you're data is unbiased and it will be sampled uniformly!" << std::endl;
std::cout << "Caution: No weights are used in association with the position data. This assumes your data is unbiased and it will be sampled uniformly!" << std::endl;

Copilot uses AI. Check for mistakes.
SamplePrimaryEnergyDistribution::SamplePrimaryEnergyDistribution(std::vector<double> energies, std::vector<double> weights)
: energies_(energies), weights_(weights) {
if (weights_.empty()) {
std::cout << "Caution: No weights are used in association with the position data. This assumes you're data is unbiased and it will be sampled uniformly!" << std::endl;

Copilot AI Dec 23, 2025

Copy link

Choose a reason for hiding this comment

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

Corrected spelling of 'you're' to 'your'. Also, the message mentions 'position data' when it should refer to 'energy data'.

Suggested change
std::cout << "Caution: No weights are used in association with the position data. This assumes you're data is unbiased and it will be sampled uniformly!" << std::endl;
std::cout << "Caution: No weights are used in association with the energy data. This assumes your data is unbiased and it will be sampled uniformly!" << std::endl;

Copilot uses AI. Check for mistakes.
ss << "position=(" << p.position.at(0) << ", " << p.position.at(1) << ", " << p.position.at(2) << "), ";
ss << "length=" << p.length << ", ";
ss << "helicity=" << p.helicity;
ss << "injection_step_length=" << p.injection_step_length;

Copilot AI Dec 23, 2025

Copy link

Choose a reason for hiding this comment

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

Missing comma separator before injection_step_length in the representation string. All other fields are separated by commas, but this one is missing the preceding comma.

Suggested change
ss << "injection_step_length=" << p.injection_step_length;
ss << ", injection_step_length=" << p.injection_step_length;

Copilot uses AI. Check for mistakes.
p.injection_step_length = GetInjectionStepLength();
} catch(...) {
p.injection_step_length = 0;
}

Copilot AI Dec 23, 2025

Copy link

Choose a reason for hiding this comment

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

Function GetParticle() is missing a return statement after the try-catch block (around line 58). The function should return p after setting the injection_step_length.

Suggested change
}
}
return p;

Copilot uses AI. Check for mistakes.
Comment thread cmake/parse_pyproject.py
try:
import tomllib # Python 3.11+
except ImportError:
except ModuleNotFoundError:

Copilot AI Dec 23, 2025

Copy link

Choose a reason for hiding this comment

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

Changed from ImportError to ModuleNotFoundError, but ModuleNotFoundError is a subclass of ImportError and only exists in Python 3.6+. This reduces backward compatibility unnecessarily.

Copilot uses AI. Check for mistakes.
Comment thread CMakeLists.txt
SET(CMAKE_CXX_STANDARD 14)
SET(CMAKE_C_STANDARD 99)

set(CMAKE_BUILD_TYPE Debug)

Copilot AI Dec 23, 2025

Copy link

Choose a reason for hiding this comment

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

Forcing CMAKE_BUILD_TYPE to Debug unconditionally (line 43) overrides user preferences and conflicts with the logic on lines 47-48 that provides a default only when not set. This should respect existing CMAKE_BUILD_TYPE settings.

Suggested change
set(CMAKE_BUILD_TYPE Debug)

Copilot uses AI. Check for mistakes.
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.

5 participants