Conversation
|
Thank you for submitting this pull request (PR)! ✨ Below are checks that are being run for this PR. Click on the name of a check to learn why it didn't pass. ✅ Don't worry if something broke! We break stuff all the time. 😅 PlasmaPy's contributor guide has pages on:
Tip 📚 For a documentation preview, click on docs/readthedocs.org:plasmapy below. For cryptic documentation errors, see the documentation troubleshooting guide. Tip 🧹 Automatically fix most pre-commit.ci failures by commenting We thank you once again! 🌌 |
github-actions
bot
added
plasmapy.plasma
|
@namurphy What do you think about the structure of |
github-actions
bot
added
docs
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3096 +/- ##
==========================================
- Coverage 95.48% 94.95% -0.53%
==========================================
Files 108 109 +1
Lines 9869 9935 +66
Branches 1501 1507 +6
==========================================
+ Hits 9423 9434 +11
- Misses 255 304 +49
- Partials 191 197 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@pheuer Thoughts on this? |
| An array of the atomic numbers of the elements making up the compound. | ||
| Does not have any units. | ||
|
|
||
| fractional_weights : `~astropy.units.Quantity` |
There was a problem hiding this comment.
I think it's much better to have these be the number fractions. When you read a chemical formula, that's what you have available, e.g. C2H4, = C:2, H:4. Calculating the weight fractions in the code is then easy using the atomic masses.
| An array of the atomic weights of the elements in the compound in units | ||
| convertible to atomic mass units. | ||
|
|
||
| Z : `~astropy.units.Quantity` |
There was a problem hiding this comment.
If A is the atomic number, then Z is usually the ionization state in a plasma. But this is a solid, so I think you only need A. In the Moliere scattering then, "z" is "A".
| The density of the material in units convertible to kilograms per cubic | ||
| meter. | ||
|
|
||
| name : str, optional |
There was a problem hiding this comment.
I think I suggested this but now IDK if we really need the name attribute
|
|
||
|
|
||
| class MaterialSTARData(TypedDict): | ||
| """Type definition for formatting data related to beam-target interactions.""" |
There was a problem hiding this comment.
In theory these are redundant - once you know the stopping power, you could calculate the others. Not saying the class has to do that, but it could in theory.
There was a problem hiding this comment.
I think it should be possible for some of these to be None - what if all you have, and all you need, is stopping power data?
| name : str, optional | ||
| The name of the material. | ||
|
|
||
| STAR_data : `~plasmapy.plasma.material.MaterialSTARData`, optional |
There was a problem hiding this comment.
I don't really like the idea of providing this as an iterable of this class in the case of different particles. How about storing this internally as a nested dict, something like d['proton']['stopping_power'], then creating methods like add_stopping_power(particle, energy_axis, data) to populate it, creating the interpolators as you go?
| stopping_power: u.Quantity[ | ||
| u.MeV * u.cm**2 / u.g | ||
| ] # Units of energy per areal density | ||
| CSDA_range: u.Quantity[u.cm] |
There was a problem hiding this comment.
How is CSDA range different than projected range? I thought all the projected range data was using the CDSA approximation.
2 participants
In this PR, I outline a consistent structure for stopping and ranging (STAR) data by defining
MaterialSTARData. I then use this definition to create an interface for users to create compounds using theMaterialclass. This is critical to my PR implementing Moliere scattering (#2748) to ensure that we provide support for users who wish to scatter through a compound