Skip to content

Change element name to fix ROOT parser #51

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
antonioiuliano2 wants to merge 2 commits into from
Closed

Change element name to fix ROOT parser #51

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a2f55c3
fix: use different names for material and elements to correctly impor…
antonioiuliano2 Jul 1, 2026
72751d7
fix: update element name in documentation string
antonioiuliano2 Jul 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file.

### Bug fixes

- *(Materials)* Use different names between material and elements to correctly import them in ROOT
- *(build)* Include GNUInstallDirs before add_subdirectory(subsystems)
- *(timing)* Install GMX data file and resolve path at runtime

Expand Down
2 changes: 1 addition & 1 deletion include/SHiPGeometry/SHiPMaterials.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class SHiPMaterials {

/**
* @brief Get an element by name
* @param name Element name (e.g., "Nitrogen", "Oxygen")
* @param name Element name (e.g., "Nitrogen_E", "Oxygen_E")
* @return Pointer to GeoElement or nullptr if not found
*/
GeoElement* getElement(const std::string& name) const;
Expand Down
164 changes: 82 additions & 82 deletions src/SHiPMaterials.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,174 +36,174 @@ GeoMaterial* SHiPMaterials::requireMaterial(const std::string& name) const {

void SHiPMaterials::createElements() {
// Create all elements needed for SHiP
m_elements["Hydrogen"] = new GeoElement(
"Hydrogen", "H", 1, 1.008 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Helium"] = new GeoElement(
"Helium", "He", 2, 4.003 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Carbon"] = new GeoElement(
"Carbon", "C", 6, 12.011 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Nitrogen"] = new GeoElement(
"Nitrogen", "N", 7, 14.007 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Oxygen"] = new GeoElement(
"Oxygen", "O", 8, 15.999 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Silicon"] = new GeoElement(
"Silicon", "Si", 14, 28.085 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Argon"] = new GeoElement(
"Argon", "Ar", 18, 39.948 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Calcium"] = new GeoElement(
"Calcium", "Ca", 20, 40.078 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Titanium"] = new GeoElement(
"Titanium", "Ti", 22, 47.867 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Chromium"] = new GeoElement(
"Chromium", "Cr", 24, 51.996 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Manganese"] = new GeoElement(
"Manganese", "Mn", 25, 54.938 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Iron"] = new GeoElement(
"Iron", "Fe", 26, 55.845 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Nickel"] = new GeoElement(
"Nickel", "Ni", 28, 58.693 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Copper"] = new GeoElement(
"Copper", "Cu", 29, 63.546 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Niobium"] = new GeoElement(
"Niobium", "Nb", 41, 92.906 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Molybdenum"] = new GeoElement(
"Molybdenum", "Mo", 42, 95.95 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Tantalum"] = new GeoElement(
"Tantalum", "Ta", 73, 180.948 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Tungsten"] = new GeoElement(
"Tungsten", "W", 74, 183.84 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Hydrogen_E"] = new GeoElement(
"Hydrogen_E", "H", 1, 1.008 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Helium_E"] = new GeoElement(
"Helium_E", "He", 2, 4.003 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Carbon_E"] = new GeoElement(
"Carbon_E", "C", 6, 12.011 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Nitrogen_E"] = new GeoElement(
"Nitrogen_E", "N", 7, 14.007 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Oxygen_E"] = new GeoElement(
"Oxygen_E", "O", 8, 15.999 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Silicon_E"] = new GeoElement(
"Silicon_E", "Si", 14, 28.085 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Argon_E"] = new GeoElement(
"Argon_E", "Ar", 18, 39.948 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Calcium_E"] = new GeoElement(
"Calcium_E", "Ca", 20, 40.078 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Titanium_E"] = new GeoElement(
"Titanium_E", "Ti", 22, 47.867 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Chromium_E"] = new GeoElement(
"Chromium_E", "Cr", 24, 51.996 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Manganese_E"] = new GeoElement(
"Manganese_E", "Mn", 25, 54.938 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Iron_E"] = new GeoElement(
"Iron_E", "Fe", 26, 55.845 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Nickel_E"] = new GeoElement(
"Nickel_E", "Ni", 28, 58.693 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Copper_E"] = new GeoElement(
"Copper_E", "Cu", 29, 63.546 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Niobium_E"] = new GeoElement(
"Niobium_E", "Nb", 41, 92.906 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Molybdenum_E"] = new GeoElement(
"Molybdenum_E", "Mo", 42, 95.95 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Tantalum_E"] = new GeoElement(
"Tantalum_E", "Ta", 73, 180.948 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Tungsten_E"] = new GeoElement(
"Tungsten_E", "W", 74, 183.84 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);

// Aluminium
m_elements["Aluminium"] = new GeoElement(
"Aluminium", "Al", 13.0, 26.982 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
m_elements["Aluminium_E"] = new GeoElement(
"Aluminium_E", "Al", 13.0, 26.982 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);

// Lead
m_elements["Lead"] = new GeoElement("Lead", "Pb", 82.0,
m_elements["Lead_E"] = new GeoElement("Lead_E", "Pb", 82.0,
207.2 * GeoModelKernelUnits::g / GeoModelKernelUnits::mole);
}
Comment on lines +39 to 83

@coderabbitai coderabbitai Bot Jul 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Verify direct callers still using documented unsuffixed element names.
rg -n -P '\bgetElement\s*\(\s*"(Hydrogen|Helium|Carbon|Nitrogen|Oxygen|Silicon|Argon|Calcium|Titanium|Chromium|Manganese|Iron|Nickel|Copper|Niobium|Molybdenum|Tantalum|Tungsten|Aluminium|Lead)"\s*\)' . || true

# Verify public docs/examples were updated with the new element naming contract.
rg -n 'Element name \(e\.g\., "Nitrogen", "Oxygen"\)|getElement\(.*Nitrogen' include/SHiPGeometry/SHiPMaterials.h || true

Repository: ShipSoft/Geometry

Length of output: 219

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the public API and nearby implementation.
ast-grep outline include/SHiPGeometry/SHiPMaterials.h --view expanded || true
ast-grep outline src/SHiPMaterials.cpp --view expanded || true

# Find all getElement call sites and the element keys used in this module.
rg -n '\bgetElement\s*\(' . || true
rg -n '"[A-Za-z]+(_E)?"' src/SHiPMaterials.cpp include/SHiPGeometry/SHiPMaterials.h || true

Repository: ShipSoft/Geometry

Length of output: 10584

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find explicit unsuffixed element lookups and any compatibility layer.
rg -n '"(Hydrogen|Helium|Carbon|Nitrogen|Oxygen|Silicon|Argon|Calcium|Titanium|Chromium|Manganese|Iron|Nickel|Copper|Niobium|Molybdenum|Tantalum|Tungsten|Aluminium|Lead)"' . || true
rg -n 'm_elements\.find|m_elements\[' src/SHiPMaterials.cpp include/SHiPGeometry/SHiPMaterials.h || true

Repository: ShipSoft/Geometry

Length of output: 9667

Keep getElement() compatible with the documented names

createElements() now stores only _E keys, but include/SHiPGeometry/SHiPMaterials.h still documents unsuffixed lookups like getElement("Nitrogen") and getElement("Oxygen"). Keep aliases for the old names or update the contract and all consumers together.

🧰 Tools
🪛 GitHub Actions: Lint / 0_prek _ prek.txt

[error] 78-80: Prek hook 'clang-format' failed. Files were modified by this hook. Run 'prek run --all-files' locally to apply clang-format changes.

🪛 GitHub Actions: Lint / prek _ prek

[error] 78-81: pre-commit hook 'clang-format' failed: files were modified by this hook. The CI run did not include formatted changes. Diff indicates clang-format reflowed the GeoElement construction for m_elements["Lead_E"].

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/SHiPMaterials.cpp` around lines 39 - 83, `createElements()` in
`SHiPMaterials` now registers only the suffixed `_E` keys, but `getElement()` is
still documented and likely used with unsuffixed names such as “Nitrogen” and
“Oxygen”. Add compatibility aliases in `createElements()` (or otherwise
normalize lookups in `getElement()`) so both the old documented names and the
new `_E` names resolve to the same `GeoElement`, and keep the public contract
consistent with existing consumers.


void SHiPMaterials::createMaterials() {
// Air (density 1.29e-3 g/cm³): N 75.5%, O 23.1%, Ar 1.4%
GeoMaterial* air =
new GeoMaterial("Air", 1.29e-3 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
air->add(m_elements["Nitrogen"], 0.755);
air->add(m_elements["Oxygen"], 0.231);
air->add(m_elements["Argon"], 0.014);
air->add(m_elements["Nitrogen_E"], 0.755);
air->add(m_elements["Oxygen_E"], 0.231);
air->add(m_elements["Argon_E"], 0.014);
air->lock();
m_materials["Air"] = air;

// Concrete (density 2.3 g/cm³): O 52%, Si 33%, Ca 15%
GeoMaterial* concrete =
new GeoMaterial("Concrete", 2.3 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
concrete->add(m_elements["Oxygen"], 0.52);
concrete->add(m_elements["Silicon"], 0.33);
concrete->add(m_elements["Calcium"], 0.15);
concrete->add(m_elements["Oxygen_E"], 0.52);
concrete->add(m_elements["Silicon_E"], 0.33);
concrete->add(m_elements["Calcium_E"], 0.15);
concrete->lock();
m_materials["Concrete"] = concrete;

// Vacuum (density 1.205e-6 g/cm³): N 75.5%, O 23.1%, Ar 1.4%
GeoMaterial* vacuum =
new GeoMaterial("Vacuum", 1.205e-6 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
vacuum->add(m_elements["Nitrogen"], 0.755);
vacuum->add(m_elements["Oxygen"], 0.231);
vacuum->add(m_elements["Argon"], 0.014);
vacuum->add(m_elements["Nitrogen_E"], 0.755);
vacuum->add(m_elements["Oxygen_E"], 0.231);
vacuum->add(m_elements["Argon_E"], 0.014);
vacuum->lock();
m_materials["Vacuum"] = vacuum;

// PressurisedHe90 (density 0.00212 g/cm³): pure He
GeoMaterial* pressurisedHe90 = new GeoMaterial(
"PressurisedHe90", 0.00212 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
pressurisedHe90->add(m_elements["Helium"], 1.0);
pressurisedHe90->add(m_elements["Helium_E"], 1.0);
pressurisedHe90->lock();
m_materials["PressurisedHe90"] = pressurisedHe90;

// Tungsten (density 19.3 g/cm³): pure W
GeoMaterial* tungsten =
new GeoMaterial("Tungsten", 19.3 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
tungsten->add(m_elements["Tungsten"], 1.0);
tungsten->add(m_elements["Tungsten_E"], 1.0);
tungsten->lock();
m_materials["Tungsten"] = tungsten;

// Silicon (density 2.33 g/cm³): pure Si (neutrino-target tracking planes)
GeoMaterial* silicon =
new GeoMaterial("Silicon", 2.33 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
silicon->add(m_elements["Silicon"], 1.0);
silicon->add(m_elements["Silicon_E"], 1.0);
silicon->lock();
m_materials["Silicon"] = silicon;

// Tantalum (density 16.65 g/cm³): pure Ta
GeoMaterial* tantalum =
new GeoMaterial("Tantalum", 16.65 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
tantalum->add(m_elements["Tantalum"], 1.0);
tantalum->add(m_elements["Tantalum_E"], 1.0);
tantalum->lock();
m_materials["Tantalum"] = tantalum;

// Inconel718 (density 8.19 g/cm³): Ni 55%, Cr 20%, Fe 16%, Nb 5%, Mo 3%, Ti 1%
GeoMaterial* inconel718 =
new GeoMaterial("Inconel718", 8.19 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
inconel718->add(m_elements["Nickel"], 0.55);
inconel718->add(m_elements["Chromium"], 0.20);
inconel718->add(m_elements["Iron"], 0.16);
inconel718->add(m_elements["Niobium"], 0.05);
inconel718->add(m_elements["Molybdenum"], 0.03);
inconel718->add(m_elements["Titanium"], 0.01);
inconel718->add(m_elements["Nickel_E"], 0.55);
inconel718->add(m_elements["Chromium_E"], 0.20);
inconel718->add(m_elements["Iron_E"], 0.16);
inconel718->add(m_elements["Niobium_E"], 0.05);
inconel718->add(m_elements["Molybdenum_E"], 0.03);
inconel718->add(m_elements["Titanium_E"], 0.01);
inconel718->lock();
m_materials["Inconel718"] = inconel718;

// Steel316L (density 7.99 g/cm³): Fe 65.3%, Cr 15.7%, Ni 12.5%, Mo 4.2%, Mn 1.9%, Si 0.4%
GeoMaterial* steel316L =
new GeoMaterial("Steel316L", 7.99 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
steel316L->add(m_elements["Iron"], 0.653);
steel316L->add(m_elements["Chromium"], 0.157);
steel316L->add(m_elements["Nickel"], 0.125);
steel316L->add(m_elements["Molybdenum"], 0.042);
steel316L->add(m_elements["Manganese"], 0.019);
steel316L->add(m_elements["Silicon"], 0.004);
steel316L->add(m_elements["Iron_E"], 0.653);
steel316L->add(m_elements["Chromium_E"], 0.157);
steel316L->add(m_elements["Nickel_E"], 0.125);
steel316L->add(m_elements["Molybdenum_E"], 0.042);
steel316L->add(m_elements["Manganese_E"], 0.019);
steel316L->add(m_elements["Silicon_E"], 0.004);
steel316L->lock();
m_materials["Steel316L"] = steel316L;

// Copper (density 8.96 g/cm³): pure Cu
GeoMaterial* copper =
new GeoMaterial("Copper", 8.96 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
copper->add(m_elements["Copper"], 1.0);
copper->add(m_elements["Copper_E"], 1.0);
copper->lock();
m_materials["Copper"] = copper;

// Iron (density 7.87 g/cm³): pure Fe
GeoMaterial* iron =
new GeoMaterial("Iron", 7.87 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
iron->add(m_elements["Iron"], 1.0);
iron->add(m_elements["Iron_E"], 1.0);
iron->lock();
m_materials["Iron"] = iron;

// Aluminium (density 2.70 g/cm³): pure Al
GeoMaterial* aluminium =
new GeoMaterial("Aluminium", 2.70 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
aluminium->add(m_elements["Aluminium"], 1.0);
aluminium->add(m_elements["Aluminium_E"], 1.0);
aluminium->lock();
m_materials["Aluminium"] = aluminium;

// LAB — linear alkylbenzene liquid scintillator (density 0.867 g/cm³):
// C 87.41%, H 12.59% by mass. Used in the Surround Background Tagger cells.
GeoMaterial* lab =
new GeoMaterial("LAB", 0.867 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
lab->add(m_elements["Carbon"], 0.8741);
lab->add(m_elements["Hydrogen"], 0.1259);
lab->add(m_elements["Carbon_E"], 0.8741);
lab->add(m_elements["Hydrogen_E"], 0.1259);
lab->lock();
m_materials["LAB"] = lab;

// Scintillator / polyvinyltoluene (density 1.023 g/cm³): C 91.5%, H 8.5%
GeoMaterial* scintillator =
new GeoMaterial("Scintillator", 1.023 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
scintillator->add(m_elements["Carbon"], 0.915);
scintillator->add(m_elements["Hydrogen"], 0.085);
scintillator->add(m_elements["Carbon_E"], 0.915);
scintillator->add(m_elements["Hydrogen_E"], 0.085);
scintillator->lock();
m_materials["Scintillator"] = scintillator;

// Lead (density 11.34 g/cm³): pure Pb
GeoMaterial* lead =
new GeoMaterial("Lead", 11.34 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
lead->add(m_elements["Lead"], 1.0);
lead->add(m_elements["Lead_E"], 1.0);
lead->lock();
m_materials["Lead"] = lead;

Expand All @@ -215,8 +215,8 @@ void SHiPMaterials::createMaterials() {
const double mw = 9.0 * awC + 10.0 * awH;
GeoMaterial* pvt =
new GeoMaterial("PVT", 1.032 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
pvt->add(m_elements["Carbon"], 9.0 * awC / mw);
pvt->add(m_elements["Hydrogen"], 10.0 * awH / mw);
pvt->add(m_elements["Carbon_E"], 9.0 * awC / mw);
pvt->add(m_elements["Hydrogen_E"], 10.0 * awH / mw);
pvt->lock();
m_materials["PVT"] = pvt;
}
Expand All @@ -229,8 +229,8 @@ void SHiPMaterials::createMaterials() {
const double mw = 8.0 * awC + 8.0 * awH;
GeoMaterial* polystyrene = new GeoMaterial(
"Polystyrene", 1.05 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
polystyrene->add(m_elements["Carbon"], 8.0 * awC / mw);
polystyrene->add(m_elements["Hydrogen"], 8.0 * awH / mw);
polystyrene->add(m_elements["Carbon_E"], 8.0 * awC / mw);
polystyrene->add(m_elements["Hydrogen_E"], 8.0 * awH / mw);
polystyrene->lock();
m_materials["Polystyrene"] = polystyrene;
}
Expand All @@ -244,9 +244,9 @@ void SHiPMaterials::createMaterials() {
const double mw = 10.0 * awC + 8.0 * awH + 4.0 * awO;
GeoMaterial* mylar =
new GeoMaterial("Mylar", 1.39 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
mylar->add(m_elements["Carbon"], 10.0 * awC / mw);
mylar->add(m_elements["Hydrogen"], 8.0 * awH / mw);
mylar->add(m_elements["Oxygen"], 4.0 * awO / mw);
mylar->add(m_elements["Carbon_E"], 10.0 * awC / mw);
mylar->add(m_elements["Hydrogen_E"], 8.0 * awH / mw);
mylar->add(m_elements["Oxygen_E"], 4.0 * awO / mw);
mylar->lock();
m_materials["Mylar"] = mylar;
}
Expand All @@ -260,9 +260,9 @@ void SHiPMaterials::createMaterials() {
const double fracCO2 = 0.30;
GeoMaterial* arco2 = new GeoMaterial(
"ArCO2_70_30", 1.56e-3 * GeoModelKernelUnits::g / GeoModelKernelUnits::cm3);
arco2->add(m_elements["Argon"], fracAr);
arco2->add(m_elements["Carbon"], fracCO2 * 12.011 / mwCO2);
arco2->add(m_elements["Oxygen"], fracCO2 * 2.0 * 15.999 / mwCO2);
arco2->add(m_elements["Argon_E"], fracAr);
arco2->add(m_elements["Carbon_E"], fracCO2 * 12.011 / mwCO2);
arco2->add(m_elements["Oxygen_E"], fracCO2 * 2.0 * 15.999 / mwCO2);
arco2->lock();
m_materials["ArCO2_70_30"] = arco2;
}
Expand Down
Loading