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

WIP: Add simple test to compute photon energy deposition #794

Conversation

helen-brooks
Copy link

Still WIP - one test is failing depending on which system I run it on.

Purpose

Ultimately the goal is to enable photon energy deposition tallies in Celeritas. The addition of this test is a step towards that goal.

New features

  • This PR adds TestPhotonCaloTest (inherits from CaloTest) to StepCollector.test.cc.
  • StepCollectorTestBase::run_impl now takes an additional optional argument num_batches, to perform a loop over several batches of tracks.
  • It was necessary to add some virtual methods to StepCollectorTestBase which are called in run_impl. This is because the Stepper initialisation should only occur once, and thus the loop over batches needs to be performed inside run_impl. However, accumulation of results needs to occur between batches, so this is now handled by generate_batch_results, with intialization and finalization of results occuring in initialize and finalize respectively.

Other changes

  • A consequence of the aforementioned changes to StepCollectorTestBase was the need to add additional state attributes and refactor the run methods within the other tests that inherit from this class (e.g. RunResults is now a member variable).

@@ -51,8 +51,13 @@ class MctruthTestBase : virtual public StepCollectorTestBase
RunResult run(size_type num_tracks, size_type num_steps);

protected:
virtual void gather_batch_results();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
virtual void gather_batch_results();
void gather_batch_results() override;

@amandalund
Copy link
Contributor

FYI I ran the test built with both ORANGE:

result.edep = {9.0653751813737, 17.177626720469, 12.691359768897}
result.edep_err = {0.64823529758419, 0.42812745087455, 0.63485083267382}

and VecGeom:

result.edep = {7.4703267105771, 13.307635626904, 12.729336876194}
result.edep_err = {0.56682089616033, 0.72393548958539, 0.65130849851177}

The ORANGE results are consistent with the expected results currently in the test, and the actual value in the failing test matches the VecGeom value, so this should explain the difference in the results.

@helen-brooks
Copy link
Author

FYI I ran the test built with both ORANGE:

result.edep = {9.0653751813737, 17.177626720469, 12.691359768897}
result.edep_err = {0.64823529758419, 0.42812745087455, 0.63485083267382}

and VecGeom:

result.edep = {7.4703267105771, 13.307635626904, 12.729336876194}
result.edep_err = {0.56682089616033, 0.72393548958539, 0.65130849851177}

The ORANGE results are consistent with the expected results currently in the test, and the actual value in the failing test matches the VecGeom value, so this should explain the difference in the results.

Oh thanks for this @amandalund ! How could I enforce using either ORANGE or VecGeom?

@sethrj
Copy link
Member

sethrj commented Jun 8, 2023

If VecGeom is configured in CMake it should be the default. You can make 100% sure by setting the cmake variable -DCELERITAS_CORE_GEO=VecGeom.

@helen-brooks
Copy link
Author

-DCELERITAS_CORE_GEO=VecGeom

It is already set to this value, and

#define CELERITAS_CORE_GEO CELERITAS_CORE_GEO_VECGEOM

is set in celeritas_config_h.
So it should already be using VecGeom. Thoughts?

@amandalund
Copy link
Contributor

Hmm, that's odd... could you try building with both VecGeom and ORANGE and print out the test results for each?

@sethrj
Copy link
Member

sethrj commented Jul 26, 2023

Closing for now but will use this as a foundation for completing #809l

@sethrj sethrj closed this Jul 26, 2023
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.

None yet

3 participants