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

Mpgd gas mix #784

Merged
merged 7 commits into from
Sep 26, 2024
Merged

Mpgd gas mix #784

merged 7 commits into from
Sep 26, 2024

Conversation

mposik1983
Copy link
Contributor

Briefly, what does this PR introduce?

Change the gas mixture in the outer barrel and end cap MPGDs

What kind of change does this PR introduce?

  • Bug fix (issue #__)
  • New feature (issue #__)
  • Documentation update
  • [ x] Other: __

Please check if this PR fulfills the following:

  • Tests for the changes have been added
  • Documentation has been added / updated
  • [ x] Changes have been communicated to collaborators

Does this PR introduce breaking changes? What changes might users need to make to their code?

No. None

Does this PR change default behavior?

Gas mixture change will lead to slightly different deposited energy distribution in the outer barrel and end cap MPGDs

@wdconinc
Copy link
Contributor

Can I suggest that the naming scheme for the gas mixtures can use improvement? Both Ar10CO2 and Ar90IsoButane contain 90% argon...

@mposik1983
Copy link
Contributor Author

Can I suggest that the naming scheme for the gas mixtures can use improvement? Both Ar10CO2 and Ar90IsoButane contain 90% argon...

I can see how that will lead to confusion. I would like to keep the Ar90IsoButane structure because this is already being used in mpgd_barrel.xml. It looks like Ar10CO2 was used sometime ago as most files show up in depreciated directory. The files not in depreciated are in central_tracker_hybrid_v2.xml, can you remind me if this is still used? I can modify those and the gas mix definition to be Ar90CO2

@wdconinc
Copy link
Contributor

Sounds good. If you just rename the Ar10CO2 gas name, then even if it is used (which I don't think it is), it shouldn't matter.

@mposik1983
Copy link
Contributor Author

Ok, I just changed the gas name in materials.xml

@wdconinc
Copy link
Contributor

Ok, I just changed the gas name in materials.xml

Well, we should also change it where it is used. Now we're in an inconsistent state (even if the files aren't used).

@mposik1983
Copy link
Contributor Author

Ok, I believe I have all cases of "Ar10CO2" --> "Ar90CO2"

Copy link
Contributor

@wdconinc wdconinc 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. Thanks for making the changes in adjacent scope!

mposik1983 and others added 5 commits September 20, 2024 12:33
change gas mixture
rename ArCO2 90/10 mixture from Ar10CO2 --> Ar90CO2
@wdconinc wdconinc added this pull request to the merge queue Sep 20, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Sep 21, 2024
@wdconinc wdconinc added this pull request to the merge queue Sep 26, 2024
Merged via the queue into main with commit a8c77be Sep 26, 2024
114 of 115 checks passed
@wdconinc wdconinc deleted the mpgd_gas_mix branch September 26, 2024 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants