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

Set SetMeanEnergyPerIonPair for acollinearity - continuation #485

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

MaxTousss
Copy link
Contributor

@MaxTousss MaxTousss commented Oct 9, 2024

To get acolin as expected in PET (i.e. ~0.5 deg FWHM) with Ion source, the mean energy per ion pair of the material where the annihilation happens need to be set to 5.0 eV. This PR create the hook from GEANT4, test the feature and document the feature. Note: this is a continuation of #439

  • Hook to GEANT4 Get/Set MeanEnergyPerIonPair set SetMeanEnergyPerIonPair for acollinearity #439
  • Test with basic material set SetMeanEnergyPerIonPair for acollinearity #439
  • Update to current version of openGate and update the test accordingly
  • Test with custom material
  • Test with material from GateMaterial
  • Test with more than one material
  • Test with e+ source (Only done with basic material, assuming that it is good enough)
  • Documentation
    • Material need to be declared with add_material_nb_atoms for non-basic material, i.e. even when defined in GateMaterials. However, this is not documented currently in the material section. Seems this was a bug, now it behave correctly.
    • Get an approval to remove the sim.physics_manager = True (See zxc in files changed)
    • How to set the mean energy per ion pair for gate/geant4/custom materials
  • Test that the value 6.0 eV approximate the results obtained with Shibuya et al 2007 (i.e., 0.55 deg FWHM) (Only done with basic material, assuming that it is good enough)
  • Sanity check if it affect positron range I did not found a test already done for positron range. Adding such a test would be outside the scope of this PR. Since the potential "problem" is documented in the user guide of this PR, this should be good enough for now.
  • Sanity check that it is the material where the annihilation occurs that need to be correctly defined. (Should be since a source does not need to be linked to a material.)

@MaxTousss
Copy link
Contributor Author

@dsarrut

Currently, the test does not pass with materials created on the go.

For example, if we use the following material for the test:

    elems = ["C", "H", "O"]
    nbAtoms = [5, 8, 2]
    gcm3 = gate.g4_units.g_cm3
    sim.volume_manager.material_database.add_material_nb_atoms(
        "IEC_PLASTIC", elems, nbAtoms, 1.18 * gcm3
    )
   wb.material = "IEC_PLASTIC"

I get an Exception: Cannot find nor build material named "IEC_PLASTIC"

This seems to be caused by mat = sim.volume_manager.find_or_build_material(wb.material) which is needed to use the material SetMeanEnergyPerIonPair.

However, if the material is also added in the GateMaterials.db file, the test pass.

So, it seems that sim.volume_manager.find_or_build_material() does not take into account the modification made by the volume_manager.material_database.add_material_nb_atoms() command. Is that intended or not?

@dsarrut
Copy link
Contributor

dsarrut commented Oct 11, 2024

@MaxTousss strange, the test "test022_half_life_ion.py" also check add_material_nb_atoms and seems to run without error. Can you check again or send me an example (by email) please ?

@MaxTousss
Copy link
Contributor Author

@dsarrut Yes, that test also use the method add_material_nb_atoms put it does not use find_or_build_material. Your first implementation of the test for SetMeanEnergyPerIon is the first test where find_or_build_material is used.

As requested, the example was sent by email. I also added the .log for good measure.

@dsarrut
Copy link
Contributor

dsarrut commented Oct 11, 2024

ok, I see the problem. Give me few days to find how to deal with it ...

Here is two workarounds:

  • solution1: ad hoc option (see test079_mean_energy_per_ion_pairs_with_new_material1.py).

  • solution2: more generic way to run function after G4 initialization and before running. More generic but more convoluted for user

  • solution3: add mean_energy_per_ion_pairs in the material definition (GateMaterialDatabase). Not done yet, but probably better.

I think the first is simpler, but the second may be useful for you to debug/tests.

@MaxTousss : acolinearity works with ion source, does it work with beta+ source ?

@MaxTousss
Copy link
Contributor Author

Are test079_mean_energy_per_ion_pairs_with_new_material1.py and test079_mean_energy_per_ion_pairs_with_new_material2.py equivalent or is there a difference in application between the two? Is "solution2" the one implemented in test079_mean_energy_per_ion_pairs_with_new_material2.py?

As for solution 3, I believe that most user would use on GateMaterials.db. As such, we should not set that variable in that file, since it would require someone to have multiple definition of each materials for when they want acolin vs not.

For now, I only refactored the tests so I could extract their difference. I still need to understand if your workaround can be simplified when the material is defined in GateMaterials.db

I have no idea if it should work with beta+. I mean, beta+ is a necessary step of Ion to gamma pair in my mind, so it should, but never tested it. I have added it to the list of todos

@dsarrut
Copy link
Contributor

dsarrut commented Oct 14, 2024

Yes I was unclear sorry : please use solution1 for the moment. The solution 2 (indeed ...new_material2.py) is only to show you how to access ionisation object once G4 is init (before the run), for debug purpose.
Also sol3 is probably not the best (and required some development), so we can forget for the moment.
And yes beta+ should be tested also, I can add it once everything else is done ;)

@MaxTousss
Copy link
Contributor Author

MaxTousss commented Oct 14, 2024

I added the remaining tests. In theory, only missing missing how to deal with Gate/Geant4/custom materials in the documentation.

Also, could you comment about the following?
Get an approval to remove the sim.physics_manager = True in the documentation (See zxc in files changed)

I think that line is no longer viable, right?

… G4Cache error obtained with previous version
@MaxTousss
Copy link
Contributor Author

Previous version of the test with multiple object resulted in ~10 of the checks per OS to fail with the error

 *** G4Exception : Cache001
      issued by : G4CacheReference<V>::Destroy
Internal fatal error. Invalid G4Cache size (requested id: 3 but cache has size: 2 Possibly client created G4Cache object in a thread and tried to delete it from another thread!

In an attempt to remove that error, I merged the two actors into one. Hope it works...

@MaxTousss MaxTousss marked this pull request as ready for review October 15, 2024 18:35
@MaxTousss
Copy link
Contributor Author

Finished everything in the TODO. All tests pass locally so I decided to put as ready for review even if github checks are on-going.

@dsarrut I do not know how to proceed from here. I guess you know who can sanity check this.

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.

3 participants