Add detector geometries: Lake Geneva, KM3NeT-ORCA, ND280, SINE, UNDINE#101
Add detector geometries: Lake Geneva, KM3NeT-ORCA, ND280, SINE, UNDINE#101austinschneider wants to merge 1 commit into
Conversation
Squashed diff for paths: - resources/detectors - resources/examples - resources/fluxes/T2K_Kaons - python/_util.py Source commits on dev/HNL_DIS_clean that contributed: 088b1cd (Nicholas Kamp): move around the HNL DIS fits files 23fa85c (Nicholas Kamp): Allow underscores in model names eb78b84 (Nicholas Kamp): _util bug fixes and KM3Net renaming Additional source commit: bea39f9 (Ming-Shau Liu): SIREN ND280+ MODULE ADDITION (#73) Co-authored-by: Ming-Shau Liu <123968200+mingshauliu@users.noreply.github.com>
d00f148 to
f15cfc2
Compare
da5daf5 to
4112bfa
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds new detector geometry/material resources and tabulated flux resources (T2K kaons), plus small utility updates to support model naming and on-demand flux file generation.
Changes:
- Add detector geometry/material resource files for SINE, UNDINE, ND280, and KM3NeT-ORCA; extend IceCube geometry with DeepCore and an enlarged fiducial shell.
- Add T2K kaon flux resource data and a
FluxCalculator.pyto generate particle-specific flux files. - Update Python utilities to allow underscores in model names and to load/generate tabulated flux files from resources.
Reviewed changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| resources/fluxes/T2K_Kaons/T2K_Kaons-v1.0/test.ipynb | Notebook used to generate/inspect flux ratios and plots. |
| resources/fluxes/T2K_Kaons/T2K_Kaons-v1.0/ratio.dat | Tabulated ratio data used for _bar scaling. |
| resources/fluxes/T2K_Kaons/T2K_Kaons-v1.0/kaon-flux-data.dat | Input kaon flux data (log-scale values). |
| resources/fluxes/T2K_Kaons/T2K_Kaons-v1.0/TOT_PLUS_NUMU.dat | Source flux table used to compute ratio data. |
| resources/fluxes/T2K_Kaons/T2K_Kaons-v1.0/TOT_MINUS_NUMUBAR.dat | Source flux table used to compute ratio data. |
| resources/fluxes/T2K_Kaons/T2K_Kaons-v1.0/FluxCalculator.py | Adds code to generate derived tabulated flux files from packaged data. |
| resources/examples/example2/PaperPlots.ipynb | Formatting/metadata cleanup in plotting notebook. |
| resources/examples/additional_paper_plots/PaperPlots.ipynb | Updates cross-section plotting/reweighting workflow and adds extra rate/diagnostic cells. |
| resources/detectors/UNDINE/UNDINE-v1/materials.dat | Adds material composition definitions for UNDINE resources. |
| resources/detectors/UNDINE/UNDINE-v1/densities.dat | Adds UNDINE geometry (air/rock/lake + cylinder detector) with densities. |
| resources/detectors/SINE/SINE-v1/materials.dat | Adds material composition definitions for SINE resources. |
| resources/detectors/SINE/SINE-v1/densities.dat | Adds SINE geometry (container panels) with densities. |
| resources/detectors/ND280/ND280-v1/materials.dat | Adds ND280 materials definitions for detailed detector composition. |
| resources/detectors/ND280/ND280-v1/densities.dat | Adds ND280 geometry layout with densities and fiducial definition. |
| resources/detectors/KM3NeTORCA/KM3NeTORCA-v1/materials.dat | Adds ORCA material definitions (water/rock/mantle/core/air). |
| resources/detectors/KM3NeTORCA/KM3NeTORCA-v1/densities.dat | Adds ORCA PREM-based geometry and cylindrical detector. |
| resources/detectors/IceCube/IceCube-v1/densities.dat | Adds DeepCore + enlarged shell and switches fiducial to enlarged shell. |
| python/_util.py | Allows underscores in model names; adds helpers to locate/generate tabulated flux files via FluxCalculator.py. |
Comments suppressed due to low confidence (1)
resources/fluxes/T2K_Kaons/T2K_Kaons-v1.0/test.ipynb:1
- This notebook is committed with execution counts and a large embedded PNG output, which bloats the repo and produces noisy diffs. Consider clearing outputs (
execution_count: null, emptyoutputs) before committing and using an output-stripping tool (e.g., pre-commitnbstripout) to keep resources lightweight.
{
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| abs_flux_dir = get_tabulated_flux_model_path(model_name,must_exist=must_exist) | ||
| # require existence of FluxCalculator.py | ||
| FluxCalculatorFile = os.path.join(abs_flux_dir,"FluxCalculator.py") | ||
| assert(os.path.isfile(FluxCalculatorFile)) | ||
| spec = importlib.util.spec_from_file_location("FluxCalculator", FluxCalculatorFile) | ||
| FluxCalculator = importlib.util.module_from_spec(spec) | ||
| sys.modules["FluxCalculator"] = FluxCalculator | ||
| spec.loader.exec_module(FluxCalculator) | ||
| flux_file = FluxCalculator.MakeFluxFile(tag,abs_flux_dir) | ||
| del sys.modules["FluxCalculator"] # remove flux directory from the system | ||
| return flux_file |
There was a problem hiding this comment.
The function body is over-indented (8 spaces) relative to the def line, which will raise an IndentationError (or create unintended nesting depending on surrounding code). Re-indent the body to a single level (4 spaces) and ensure consistent formatting with the rest of the module.
| abs_flux_dir = get_tabulated_flux_model_path(model_name,must_exist=must_exist) | |
| # require existence of FluxCalculator.py | |
| FluxCalculatorFile = os.path.join(abs_flux_dir,"FluxCalculator.py") | |
| assert(os.path.isfile(FluxCalculatorFile)) | |
| spec = importlib.util.spec_from_file_location("FluxCalculator", FluxCalculatorFile) | |
| FluxCalculator = importlib.util.module_from_spec(spec) | |
| sys.modules["FluxCalculator"] = FluxCalculator | |
| spec.loader.exec_module(FluxCalculator) | |
| flux_file = FluxCalculator.MakeFluxFile(tag,abs_flux_dir) | |
| del sys.modules["FluxCalculator"] # remove flux directory from the system | |
| return flux_file | |
| abs_flux_dir = get_tabulated_flux_model_path(model_name,must_exist=must_exist) | |
| # require existence of FluxCalculator.py | |
| FluxCalculatorFile = os.path.join(abs_flux_dir,"FluxCalculator.py") | |
| assert(os.path.isfile(FluxCalculatorFile)) | |
| spec = importlib.util.spec_from_file_location("FluxCalculator", FluxCalculatorFile) | |
| FluxCalculator = importlib.util.module_from_spec(spec) | |
| sys.modules["FluxCalculator"] = FluxCalculator | |
| spec.loader.exec_module(FluxCalculator) | |
| flux_file = FluxCalculator.MakeFluxFile(tag,abs_flux_dir) | |
| del sys.modules["FluxCalculator"] # remove flux directory from the system | |
| return flux_file |
| def bar_scaling(abs_flux_dir, m_n_min=0.002, m_n_max=0.4, m_n_step=25): | ||
|
|
||
|
|
||
| # Energy in GeV | ||
| m_n, ratio = np.loadtxt(os.path.join(abs_flux_dir,'ratio.dat'), usecols=(0, 1), unpack=True) | ||
|
|
||
| extended_m_n = np.arange(m_n_min, m_n_max + m_n_step, m_n_step) |
There was a problem hiding this comment.
np.arange(m_n_min, m_n_max + m_n_step, m_n_step) with the default m_n_step=25 and m_n_max=0.4 produces an array with (effectively) a single point, so bar_scale(...) becomes nearly constant and the scaling will be wrong. Use a step size consistent with the ratio.dat domain resolution (or rename the parameter to reflect it), e.g., a small float step, or derive the extended grid from the input m_n spacing.
| def bar_scaling(abs_flux_dir, m_n_min=0.002, m_n_max=0.4, m_n_step=25): | |
| # Energy in GeV | |
| m_n, ratio = np.loadtxt(os.path.join(abs_flux_dir,'ratio.dat'), usecols=(0, 1), unpack=True) | |
| extended_m_n = np.arange(m_n_min, m_n_max + m_n_step, m_n_step) | |
| def bar_scaling(abs_flux_dir, m_n_min=0.002, m_n_max=0.4, m_n_step=None): | |
| # Energy in GeV | |
| m_n, ratio = np.loadtxt(os.path.join(abs_flux_dir,'ratio.dat'), usecols=(0, 1), unpack=True) | |
| if m_n_step is None: | |
| m_n_diffs = np.diff(m_n) | |
| positive_diffs = m_n_diffs[m_n_diffs > 0] | |
| if positive_diffs.size > 0: | |
| m_n_step = positive_diffs.min() | |
| else: | |
| m_n_step = max(m_n_max - m_n_min, np.finfo(float).eps) | |
| else: | |
| m_n_step = float(m_n_step) | |
| if m_n_step <= 0: | |
| raise ValueError("m_n_step must be positive") | |
| extended_m_n = np.arange(m_n_min, m_n_max + m_n_step * 0.5, m_n_step) |
| def bar_scaling(abs_flux_dir, m_n_min=0.002, m_n_max=0.4, m_n_step=25): | ||
|
|
||
|
|
||
| # Energy in GeV | ||
| m_n, ratio = np.loadtxt(os.path.join(abs_flux_dir,'ratio.dat'), usecols=(0, 1), unpack=True) |
There was a problem hiding this comment.
np.arange(m_n_min, m_n_max + m_n_step, m_n_step) with the default m_n_step=25 and m_n_max=0.4 produces an array with (effectively) a single point, so bar_scale(...) becomes nearly constant and the scaling will be wrong. Use a step size consistent with the ratio.dat domain resolution (or rename the parameter to reflect it), e.g., a small float step, or derive the extended grid from the input m_n spacing.
| def bar_scaling(abs_flux_dir, m_n_min=0.002, m_n_max=0.4, m_n_step=25): | |
| # Energy in GeV | |
| m_n, ratio = np.loadtxt(os.path.join(abs_flux_dir,'ratio.dat'), usecols=(0, 1), unpack=True) | |
| def bar_scaling(abs_flux_dir, m_n_min=0.002, m_n_max=0.4, m_n_step=None): | |
| # Energy in GeV | |
| m_n, ratio = np.loadtxt(os.path.join(abs_flux_dir,'ratio.dat'), usecols=(0, 1), unpack=True) | |
| if m_n.size < 2: | |
| raise ValueError("ratio.dat must contain at least two mass points to build bar scaling") | |
| if m_n_step is None: | |
| m_n_diffs = np.diff(m_n) | |
| positive_diffs = m_n_diffs[m_n_diffs > 0] | |
| if positive_diffs.size == 0: | |
| raise ValueError("ratio.dat must contain increasing mass values to derive m_n_step") | |
| m_n_step = np.min(positive_diffs) |
| print(f'Enhance tag "{tag}" is invalid') | ||
| exit(0) | ||
|
|
||
| if particle not in ["numu", "numubar"]: | ||
| print("%s particle specified in tag %s is not valid"%(particle)) | ||
| exit(0) |
There was a problem hiding this comment.
Calling exit(0) inside a library-style helper can terminate the entire host process (including downstream tools/tests) and makes failures hard to handle. Prefer raising a typed exception (e.g., ValueError) with a clear message so callers (like python/_util.py) can catch and report it.
| print(f'Enhance tag "{tag}" is invalid') | |
| exit(0) | |
| if particle not in ["numu", "numubar"]: | |
| print("%s particle specified in tag %s is not valid"%(particle)) | |
| exit(0) | |
| raise ValueError(f'Enhance tag "{tag}" is invalid') | |
| if particle not in ["numu", "numubar"]: | |
| raise ValueError('%s particle specified in tag %s is not valid' % (particle, tag)) |
|
|
||
|
|
||
| def get_tabulated_flux_model_path(model_name, must_exist=True): | ||
| return _get_model_path(model_name,prefix="Fluxes", is_file=False, must_exist=must_exist) |
There was a problem hiding this comment.
The resources path shown in this PR is resources/fluxes/... (lowercase), but this helper uses prefix=\"Fluxes\". On case-sensitive filesystems this can prevent locating flux models. Align the prefix argument with the actual on-disk directory naming/convention used by _get_base_directory (e.g., \"fluxes\" if it maps directly to resources/fluxes).
| return _get_model_path(model_name,prefix="Fluxes", is_file=False, must_exist=must_exist) | |
| return _get_model_path(model_name,prefix="fluxes", is_file=False, must_exist=must_exist) |
| @@ -0,0 +1,25 @@ | |||
| # Material model file | |||
| # Detector: LakeGeneva | |||
There was a problem hiding this comment.
The header says Detector: LakeGeneva, but this file sits under resources/detectors/UNDINE/.... This looks like a copy/paste header mismatch and can confuse downstream users auditing detector configs. Update the header detector name to UNDINE (and similarly audit other headers in this PR for correct detector naming).
| # Detector: LakeGeneva | |
| # Detector: UNDINE |
| @@ -0,0 +1,25 @@ | |||
| # Material model file | |||
| # Detector: SurfaceGeneva | |||
There was a problem hiding this comment.
The header says Detector: SurfaceGeneva, but this file is under resources/detectors/SINE/.... If this is intended to be the SINE detector, update the header to match SINE to avoid ambiguity when these resources are consumed.
| # Detector: SurfaceGeneva | |
| # Detector: SINE |
| # Detector: UNDINE (UNDerwater Integrated Neutrino Detector) | ||
| # Version: v1 | ||
| # Material model file: UNDINE_LHCb_North-v1.dat | ||
| # Date: 2024-10-3 |
There was a problem hiding this comment.
Date formatting is inconsistent (2024-10-3 vs YYYY-MM-DD used elsewhere). For consistency and easier parsing/validation, use zero-padded dates (e.g., 2024-10-03).
| # Date: 2024-10-3 | |
| # Date: 2024-10-03 |
| LEAD 3 | ||
| 1000822080 0.77528090 # Pb208 | ||
| 1000260560 0.20224719 # Fe56 | ||
| 1000240520 0.02247191 # Fe56 |
There was a problem hiding this comment.
Line 93's isotope code 1000240520 corresponds to Z=24 (Cr), but the comment says Fe56. Either the isotope code or the comment is wrong; please correct this to avoid incorrect interpretation of the material composition (and to prevent accidental physics mistakes if someone later updates values based on comments).
| 1000240520 0.02247191 # Fe56 | |
| 1000240520 0.02247191 # Cr52 |
| " primary_type = numubar if \"bar\" in k else numu\n", | ||
| " plt.plot(erange,[primary_xs[k].GetCrossSectionsForTarget(target_type)[0].TotalCrossSection(primary_type,e) for e in erange],label=labels[k])\n", | ||
| "si_xsfiledir = siren.utilities.get_cross_section_model_path(cross_section_model)\n", | ||
| "li_xsfiledir = \"/cvmfs/icecube.opensciencegrid.org/data/neutrino-generator/cross_section_data/csms_differential_v1.0/\"\n", |
There was a problem hiding this comment.
li_xsfiledir is defined but never used (the loop iterates only over [si_xsfiledir]). If the intent is to compare SIREN-packaged vs CVMFS tables, consider including li_xsfiledir in the loop or removing it to avoid dead code in the example notebook.
| "li_xsfiledir = \"/cvmfs/icecube.opensciencegrid.org/data/neutrino-generator/cross_section_data/csms_differential_v1.0/\"\n", |
4112bfa to
4a7e297
Compare
f15cfc2 to
8259988
Compare
4a7e297 to
e68be1a
Compare
8259988 to
c7a4994
Compare
e68be1a to
b179a08
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 3 of 14
This PR is part of a 14-PR stack decomposing
dev/HNL_DISinto reviewable chunks.chore/ci-and-packagingfeat/detectors-resourcesMerge order:
chore/ci-and-packagingshould land before this PR.Squashed contents
Squashed diff for paths:
Source commits on dev/HNL_DIS_clean that contributed:
088b1cd (Nicholas Kamp): move around the HNL DIS fits files
23fa85c (Nicholas Kamp): Allow underscores in model names
eb78b84 (Nicholas Kamp): _util bug fixes and KM3Net renaming
Notes
dev/HNL_DIS_clean..fitsdata files have been removed from the branch and are packaged separately.