Repository navigation
Add ion species enum - #4730
Add ion species enum#4730chris-ashe wants to merge 22 commits into
Conversation
…d atomic mass attributes
…ion for species attributes
…r impurity species attributes
…tency across modules
…ation for consistency across plotting scripts
…n for improved clarity and consistency
…nSpecies enumeration for improved consistency
…ion for improved error handling
…attribute and streamline data structures
…ated tests for improved clarity
…aintainability and readability
…improved clarity and consistency
…s for improved clarity and maintainability
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4730 +/- ##
==========================================
+ Coverage 50.36% 50.37% +0.01%
==========================================
Files 181 181
Lines 30666 30683 +17
==========================================
+ Hits 15444 15456 +12
- Misses 15222 15227 +5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
timothy-nunn
left a comment
timothy-nunn
left a comment
There was a problem hiding this comment.
A few comments on the naming of certain enum fields. I think some stuff could be redefined to simplify the indexing.
Certain things could be added as a method on the enum to reduce duplication.
| for species in (species for species in IonSpecies if species.is_impurity): | ||
| ind = species.value - 1 |
There was a problem hiding this comment.
| for species in (species for species in IonSpecies if species.is_impurity): | |
| ind = species.value - 1 | |
| for species in IonSpecies: | |
| if species.is_impurity: | |
| continue | |
| ind = species.value - 1 |
Reading what is written currently confused me greatly... too many species
| for species in (species for species in IonSpecies if species.is_impurity): | ||
| ind = species.value - 1 | ||
| if imp_frac[ind] > 1.0e-30: | ||
| lbl = species.label.replace("_", "") |
There was a problem hiding this comment.
Could this be added as a method on the enum? e.g. species.symbol()
| rho, | ||
| pden_rad_array[ind] * 1.0e-6, | ||
| label=imp_labels[ind].replace("_", ""), | ||
| label=species.label.replace("_", ""), |
There was a problem hiding this comment.
This could be replaced by the aforementioned method
| if not species.is_impurity: | ||
| continue | ||
|
|
||
| ind = species.value - 1 |
There was a problem hiding this comment.
We also do this calculation a lot. Could this also be a method to avoid someone making a mistake somewhere and forgetting the -1?
…on for improved accuracy and consistency
…stency and clarity across plotting functions
…larity and efficiency
…clarity and consistency across plotting functions
| if imp_frac[ind] > 1.0e-30: | ||
| for species in IonSpecies: | ||
| if species.is_impurity and imp_frac[species.index] > 1.0e-30: | ||
| lbl = species.symbol |
There was a problem hiding this comment.
species.symbol can be used directly below, it doesn't need to be assigned to a variable
| # find impurity densities | ||
| imp_frac = np.array([ | ||
| mfile.get(f"f_nd_impurity_electrons({i:02d})", scan=scan) for i in range(1, 15) | ||
| mfile.get(f"f_nd_impurity_electrons({i:02d})", scan=scan) |
There was a problem hiding this comment.
| mfile.get(f"f_nd_impurity_electrons({i:02d})", scan=scan) | |
| mfile.get(f"f_nd_impurity_electrons({i+1:02d})", scan=scan) |
There was a problem hiding this comment.
This will break the indexing I think
|
|
||
| impurity_frac = self.data.impurity_radiation.f_nd_impurity_electron_array[ | ||
| self.data.reinke.impvardiv - 1 | ||
| self.data.reinke.impvardiv |
There was a problem hiding this comment.
Was this changed elsewhere in the code or was it a pre-existing bug?
There was a problem hiding this comment.
The indexes now start at 0 so all values are down shifted
There was a problem hiding this comment.
This file still has some uses of IonSpecies.value, can these be replaced with .index?
…acy and consistency in radiation calculations
…ion for improved clarity and consistency in radiation profile plotting
Description
Checklist
I confirm that I have completed the following checks: