Skip to content

Reorganize HNL DIS spline directories under resources/processes#102

Closed
austinschneider wants to merge 1 commit into
feat/detectors-resourcesfrom
chore/file-reorg
Closed

Reorganize HNL DIS spline directories under resources/processes#102
austinschneider wants to merge 1 commit into
feat/detectors-resourcesfrom
chore/file-reorg

Conversation

@austinschneider

Copy link
Copy Markdown
Collaborator

Stack: PR 4 of 14

This PR is part of a 14-PR stack decomposing dev/HNL_DIS into reviewable chunks.

  • Base: feat/detectors-resources
  • Head: chore/file-reorg

Merge order: feat/detectors-resources should land before this PR.

Squashed contents

Squashed diff for paths:

  • resources/processes/HNLDISSplines
  • resources/processes/DipoleHNLDISSplines
  • vendor/cereal
  • vendor/delabella
  • vendor/photospline
  • vendor/pybind11

Source commits on dev/HNL_DIS_clean that contributed:
088b1cd (Nicholas Kamp): move around the HNL DIS fits files
503f918 (Nicholas Kamp): add the processes.py for the HNL splines

Notes

  • This branch is the squashed cumulative diff of the listed source commits. Per-commit history is browsable on dev/HNL_DIS_clean.
  • Large .fits data files have been removed from the branch and are packaged separately.

Squashed diff for paths:
  - resources/processes/HNLDISSplines
  - resources/processes/DipoleHNLDISSplines
  - vendor/cereal
  - vendor/delabella
  - vendor/photospline
  - vendor/pybind11

Source commits on dev/HNL_DIS_clean that contributed:
  088b1cd (Nicholas Kamp): move around the HNL DIS fits files
  503f918 (Nicholas Kamp): add the processes.py for the HNL splines
Copilot AI review requested due to automatic review settings April 28, 2026 04:28
@austinschneider austinschneider force-pushed the feat/detectors-resources branch from da5daf5 to 4112bfa Compare April 28, 2026 04:34

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

Note

Copilot was unable to run its full agentic suite in this review.

Reorganizes HNL DIS spline resources under resources/processes/* and adds Python helpers for loading spline-based interaction processes.

Changes:

  • Adds processes.py loaders for HNLDIS and Dipole HNLDIS spline bundles (v1.0/v2.0).
  • Adds Dipole HNLDIS spline .dat resources for M_0001000MeV.
  • Moves/organizes resources into versioned directories under resources/processes/.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
resources/processes/HNLDISSplines/HNLDISSplines-v2.0/processes.py Adds loader helpers for HNLDIS spline processes (v2.0).
resources/processes/HNLDISSplines/HNLDISSplines-v1.0/processes.py Adds loader helpers for HNLDIS spline processes (v1.0).
resources/processes/DipoleHNLDISSplines/DipoleHNLDISSpline-v2.0/processes.py Adds loader helpers for dipole HNLDIS spline processes (v2.0).
resources/processes/DipoleHNLDISSplines/DipoleHNLDISSpline-v1.0/processes.py Adds loader helpers for dipole HNLDIS spline processes (v1.0).
resources/processes/DipoleHNLDISSplines/DipoleHNLDISSpline-v1.0/M_0001000MeV/sigma-nubar-N-nc-GRV98lo_patched_central.dat Adds tabulated dipole cross-section data for nubar, NC.
resources/processes/DipoleHNLDISSplines/DipoleHNLDISSpline-v1.0/M_0001000MeV/sigma-nubar-N-em-GRV98lo_patched_central.dat Adds tabulated dipole cross-section data for nubar, EM.
resources/processes/DipoleHNLDISSplines/DipoleHNLDISSpline-v1.0/M_0001000MeV/sigma-nu-N-nc-GRV98lo_patched_central.dat Adds tabulated dipole cross-section data for nu, NC.
resources/processes/DipoleHNLDISSplines/DipoleHNLDISSpline-v1.0/M_0001000MeV/sigma-nu-N-em-GRV98lo_patched_central.dat Adds tabulated dipole cross-section data for nu, EM.

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

Comment on lines +63 to +74
def _get_process_types(process_types):
if process_types is None:
process_types = ["CC", "NC"]

for i, p in enumerate(process_types):
if p not in processes:
raise ValueError(f"process_types[{i}] \"{p}\" not supported. Allowed proccesses are {processes}")

if len(process_types) == 0:
print("Warning: len(process_types) == 0")

return process_types

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

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

_get_process_types references processes, but processes is not defined in this module, so calling this helper will raise NameError. Either define an allowed-processes collection (e.g., processes = {'CC','NC'}) in this file or remove this helper if process selection isn’t supported here.

Copilot uses AI. Check for mistakes.
Comment on lines +93 to +94
m4_str = f"{int(m4_MeV):07d}"
m4_GeV = m4_GeV = float(m4_MeV)*1e-3

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

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

m4_MeV defaults to None but is immediately used in int(m4_MeV) / float(m4_MeV), which will raise TypeError when the caller relies on defaults. Make m4_MeV a required argument (no default) or explicitly validate and raise a clear ValueError when it’s missing. Also, the m4_GeV = m4_GeV = ... double-assignment should be corrected.

Suggested change
m4_str = f"{int(m4_MeV):07d}"
m4_GeV = m4_GeV = float(m4_MeV)*1e-3
if m4_MeV is None:
raise ValueError("m4_MeV must be provided")
m4_str = f"{int(m4_MeV):07d}"
m4_GeV = float(m4_MeV) * 1e-3

Copilot uses AI. Check for mistakes.
Comment on lines +104 to +105
dxs_file = os.path.join(base_path, f"M_0000000MeV/dsdxdy-nu-N-nc-GRV98lo_patched_central.fits")
xs_file = os.path.join(base_path, f"M_{m4_str}MeV/sigma-nu-N-nc-GRV98lo_patched_central.fits")

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

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

nunubar is computed but not used when building filenames: both dxs_file and xs_file are hardcoded to nu. This will load neutrino splines even when primary is an antineutrino. Use nunubar in the filenames (consistent with the non-dipole loader) so nubar primaries load ...-nubar-... resources.

Suggested change
dxs_file = os.path.join(base_path, f"M_0000000MeV/dsdxdy-nu-N-nc-GRV98lo_patched_central.fits")
xs_file = os.path.join(base_path, f"M_{m4_str}MeV/sigma-nu-N-nc-GRV98lo_patched_central.fits")
dxs_file = os.path.join(base_path, f"M_0000000MeV/dsdxdy-{nunubar}-N-nc-GRV98lo_patched_central.fits")
xs_file = os.path.join(base_path, f"M_{m4_str}MeV/sigma-{nunubar}-N-nc-GRV98lo_patched_central.fits")

Copilot uses AI. Check for mistakes.
isoscalar = True

if not isoscalar:
raise ValueError("Non-isoscalar splines are not supported for CSMSDISSplines-v1.0")

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

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

The error message references CSMSDISSplines-v1.0, but this module is under HNLDISSplines/HNLDISSplines-v1.0. Updating the message to the correct spline family/version will make failures easier to diagnose (same issue appears in the v2.0 and dipole variants).

Suggested change
raise ValueError("Non-isoscalar splines are not supported for CSMSDISSplines-v1.0")
raise ValueError("Non-isoscalar splines are not supported for HNLDISSplines-v1.0")

Copilot uses AI. Check for mistakes.

for i, p in enumerate(process_types):
if p not in processes:
raise ValueError(f"process_types[{i}] \"{p}\" not supported. Allowed proccesses are {processes}")

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

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

Correct spelling in the error message: 'proccesses' → 'processes'.

Suggested change
raise ValueError(f"process_types[{i}] \"{p}\" not supported. Allowed proccesses are {processes}")
raise ValueError(f"process_types[{i}] \"{p}\" not supported. Allowed processes are {processes}")

Copilot uses AI. Check for mistakes.
Comment on lines +2 to +4
from typing import Tuple, List, Any, Optional
import siren
import collections

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

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

This module defines several unused items: Tuple/Any imports, neutrino_types/antineutrino_types, and primary_processes. Since these are new files and the full module is shown in the diff, consider removing unused imports/variables to keep the loader minimal and reduce confusion (same pattern appears in the v1.0 and dipole variants).

Suggested change
from typing import Tuple, List, Any, Optional
import siren
import collections
from typing import List, Optional
import siren

Copilot uses AI. Check for mistakes.
Comment on lines +90 to +91
neutrino_types = [t for t in primary_types if t in neutrinos]
antineutrino_types = [t for t in primary_types if t in antineutrinos]

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

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

This module defines several unused items: Tuple/Any imports, neutrino_types/antineutrino_types, and primary_processes. Since these are new files and the full module is shown in the diff, consider removing unused imports/variables to keep the loader minimal and reduce confusion (same pattern appears in the v1.0 and dipole variants).

Copilot uses AI. Check for mistakes.
m4_str = f"{int(m4_MeV):07d}"
m4_GeV = m4_GeV = float(m4_MeV)*1e-3

primary_processes = []

Copilot AI Apr 28, 2026

Copy link

Choose a reason for hiding this comment

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

This module defines several unused items: Tuple/Any imports, neutrino_types/antineutrino_types, and primary_processes. Since these are new files and the full module is shown in the diff, consider removing unused imports/variables to keep the loader minimal and reduce confusion (same pattern appears in the v1.0 and dipole variants).

Copilot uses AI. Check for mistakes.
@austinschneider

Copy link
Copy Markdown
Collaborator Author

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.

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