Skip to content
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.

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

Export / import Geant4 XS tables, volume, material, and element information #55

Merged
merged 2 commits into from
Oct 29, 2020

Conversation

stognini
Copy link
Member

Add functionality to geant-exporter app to export XS tables, material, element, and volume information into a root file.
Add capability for Celeritas to import data from the root file, as well as retrieve volume, material, element, and XS table information as needed.

The import tool will run once and the imported data will be moved into future host/device classes, similar to the current ParticleParams.

@dalg24-jenkins
Copy link

Can one of the admins verify this patch?

Copy link
Member

@sethrj sethrj left a comment

Choose a reason for hiding this comment

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

Okeydoke, first round of comments coming in. Good work and functionality overall.

app/geant-exporter/DetectorConstruction.cc Outdated Show resolved Hide resolved
app/geant-exporter/DetectorConstruction.cc Outdated Show resolved Hide resolved
app/geant-exporter/DetectorConstruction.cc Outdated Show resolved Hide resolved
app/geant-exporter/GeantPhysicsTableWriter.cc Outdated Show resolved Hide resolved
app/geant-exporter/GeantPhysicsTableWriter.hh Outdated Show resolved Hide resolved
src/io/GeantPhysicsVectorType.hh Outdated Show resolved Hide resolved
src/io/GeantProcess.hh Outdated Show resolved Hide resolved
src/io/GeantTableType.hh Outdated Show resolved Hide resolved
test/io/GeantImporter.test.cc Outdated Show resolved Hide resolved
test/io/GeantImporter.test.cc Outdated Show resolved Hide resolved
Copy link
Member

@sethrj sethrj left a comment

Choose a reason for hiding this comment

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

Excellent work, Stefano -- this is really coming together. I've got a few more comments on the changes.

app/geant-exporter/DetectorConstruction.cc Show resolved Hide resolved
app/geant-exporter/GeantPhysicsTableWriter.cc Outdated Show resolved Hide resolved
app/geant-exporter/GeantPhysicsTableWriter.cc Outdated Show resolved Hide resolved
app/geant-exporter/GeantPhysicsTableWriter.cc Outdated Show resolved Hide resolved
app/geant-exporter/GeantPhysicsTableWriter.hh Outdated Show resolved Hide resolved
app/geant-exporter/geant-exporter.cc Outdated Show resolved Hide resolved
app/geant-exporter/geant-exporter.cc Outdated Show resolved Hide resolved
src/io/GeantGeometryMap.cc Outdated Show resolved Hide resolved
src/io/GeantGeometryMap.cc Outdated Show resolved Hide resolved
src/io/GeantMaterial.hh Outdated Show resolved Hide resolved
Copy link
Member

@sethrj sethrj left a comment

Choose a reason for hiding this comment

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

Looking really good, I think this'll be the final iteration!

app/geant-exporter/GeantPhysicsTableWriter.cc Outdated Show resolved Hide resolved
app/geant-exporter/GeantPhysicsTableWriter.cc Outdated Show resolved Hide resolved
app/geant-exporter/GeantPhysicsTableWriter.hh Outdated Show resolved Hide resolved
src/io/GeantGeometryMap.cc Outdated Show resolved Hide resolved
@stognini stognini requested a review from sethrj October 28, 2020 19:04
sethrj
sethrj previously approved these changes Oct 28, 2020
Copy link
Member

@sethrj sethrj left a comment

Choose a reason for hiding this comment

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

Excellent work Stefano! I'm going to build this on one or two configurations just to make sure everything works as expected, and will then merge.

@sethrj
Copy link
Member

sethrj commented Oct 28, 2020

@stognini Hey, I'm seeing a few "warnings-as-errors" on emmet: basically that REQUIRE(&table); is pointless because if it's undefined behavior if table is a reference to a null pointer (disallowed by the C++ standard). Do you compile with the all/extra/pedantic/error flags? I'll fix the warning, squash the commits, and merge.

Geant-exporter app now exports material and volume information
using the GeantMaterialTable class.
RootImporter loads data from the ROOT file into memory.

The ImportMaterialTable class maps volume ids with their respective
material ids, as well as providing methods to retrieve ImportMaterial
and ImportVolume data. The material id allows finding the XS table
of said material stored in ImportPhysicsTable objects.
@sethrj sethrj removed the request for review from whokion October 29, 2020 00:01
@stognini
Copy link
Member Author

I'm seeing a few "warnings-as-errors" on emmet

Damn, ok. I can fix them if you prefer.

Do you compile with the all/extra/pedantic/error flags?

I could not fix a compiler problem that appeared with that flag, so I disabled it until I could fix it. I'll try to get that running correctly (and compile things on emmet for the time being) to avoid future missing warnings.

@sethrj sethrj merged commit a6b4225 into master Oct 29, 2020
@sethrj sethrj deleted the g4importer-tables branch October 29, 2020 00:22
@sethrj
Copy link
Member

sethrj commented Oct 29, 2020

No problem, already done (and I squashed a lot of the work-in-progress-y commits to a very few). I'd be curious if you can reproduce the compiler problem; that small warning was the only thing I saw on my build.

@sethrj sethrj added the material label Feb 6, 2021
@sethrj sethrj added io external Dependencies and framework-oriented features and removed material labels Feb 18, 2023
@sethrj sethrj added enhancement New feature or request and removed io labels Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request external Dependencies and framework-oriented features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants