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

Add ScintillationGenerator and related data structures #1086

Merged
merged 18 commits into from
Jan 26, 2024

Conversation

whokion
Copy link
Contributor

@whokion whokion commented Jan 19, 2024

Add an initial set of codes for generating scintillation photons and related data structures.

This MR adds an initial/minimal set of codes for generating scintillation photons and related data structure - following the earlier work by @amandalund for CerenvkovGenerator as closely as possible. Nevertheless, a significant refactoring may be foreseen to consolidate the common code-basis and accommodate future developments for high-level Cerenkov and Scintillation processes.:

  • Implement ScintillationGenerator [enhancement, physics]
  • Add ScintillationData and ScintillationParams to manage related optical property data
  • Add a rudimentary test, ScintillationGenerator.test.

@whokion whokion added enhancement New feature or request physics Particles, processes, and stepping algorithms labels Jan 19, 2024
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 good, @whokion !

src/celeritas/optical/OpticalPropertyData.hh Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationData.hh Show resolved Hide resolved
src/celeritas/optical/ScintillationGenerator.hh Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationGenerator.hh Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationGenerator.hh Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationGenerator.hh Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationGenerator.hh Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationInput.hh Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationParams.cc Outdated Show resolved Hide resolved
Copy link
Contributor

@amandalund amandalund left a comment

Choose a reason for hiding this comment

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

Looks good @whokion! I have a few higher-level questions/comments before I look at the details of the generator.

src/celeritas/optical/ScintillationInput.hh Outdated Show resolved Hide resolved
src/celeritas/optical/OpticalPropertyData.hh Outdated Show resolved Hide resolved
test/celeritas/optical/ScintillationGenerator.test.cc Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationGenerator.hh Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationGenerator.hh Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationGenerator.hh Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationGenerator.hh Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationGenerator.hh Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationGenerator.hh Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationParams.cc Show resolved Hide resolved
@whokion
Copy link
Contributor Author

whokion commented Jan 23, 2024

@sethrj, @amandalund, @stognini I do not have a computer node to work on today and will address the rest of review comments later. Sorry about being delayed.

@whokion
Copy link
Contributor Author

whokion commented Jan 24, 2024

Not sure whether I addressed all review comments (seems that some of earlier comments are disappeared), but please let me know if there are additional comments. Thanks!

Copy link
Contributor

@amandalund amandalund left a comment

Choose a reason for hiding this comment

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

Looks great @whokion! Just a few more small comments from me.

src/celeritas/optical/ScintillationGenerator.hh Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationParams.cc Show resolved Hide resolved
test/celeritas/optical/ScintillationGenerator.test.cc Outdated Show resolved Hide resolved
@whokion
Copy link
Contributor Author

whokion commented Jan 25, 2024

@sethrj Not sure what the 1 pending check is, but I guess that all review comments are more or less addressed and this MR may be good enough to be merged now. Thanks you all for your review comments and good suggestions!

Copy link
Contributor

@amandalund amandalund left a comment

Choose a reason for hiding this comment

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

Very nice, thanks @whokion! Looks good on my end.

src/celeritas/optical/ScintillationParams.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.

Nice work @whokion !

src/celeritas/optical/ScintillationGenerator.hh Outdated Show resolved Hide resolved
test/celeritas/optical/ScintillationGenerator.test.cc Outdated Show resolved Hide resolved
src/celeritas/optical/ScintillationParams.cc Show resolved Hide resolved
@whokion
Copy link
Contributor Author

whokion commented Jan 25, 2024

@sethrj All done. Thanks!

@sethrj sethrj enabled auto-merge (squash) January 26, 2024 02:18
@sethrj sethrj changed the title Add ScintillationGenerator and related data structures. Add ScintillationGenerator and related data structures Jan 26, 2024
@sethrj sethrj mentioned this pull request Jan 26, 2024
25 tasks
@sethrj
Copy link
Member

sethrj commented Jan 26, 2024

Nice work! Cross referencing with #886 .

@sethrj sethrj merged commit ff984ca into celeritas-project:develop Jan 26, 2024
19 of 20 checks passed
@whokion whokion deleted the scintillation_generator branch January 26, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request physics Particles, processes, and stepping algorithms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants