-
-
Notifications
You must be signed in to change notification settings - Fork 421
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
[TEP007] Isotope uniform config option #762
Conversation
I think in the config we should merge abundances and isotope abundances. |
@wkerzendorf Please review . |
tardis/model/base.py
Outdated
for element_symbol_string in abundances_section: | ||
if element_symbol_string == 'type': | ||
continue | ||
z = element_symbol2atomic_number(element_symbol_string) | ||
abundance.ix[z] = float(abundances_section[element_symbol_string]) | ||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe this splitting - might be better performed earlier in the process and not in the init of the model (generally keeping logic out of inits is good practice).
tardis/io/model_reader.py
Outdated
z = element_symbol2atomic_number(element_symbol_string) | ||
abundance.ix[z] = float( | ||
abundances_section[element_symbol_string]) | ||
except MalformedElementSymbolError: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what happens when this fails as well. Actually you can probably use pyne for both. try that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wkerzendorf If this fails, then it will throw error raised by pyne
.
(Ex- RuntimeError: Not a Nuclide! NIEFEG --> 0
, where Niefeg is some wrong element defined in config)
If I use pyne
then I can replace this by something like this:
try:
find atomic_no and mass_no by pyne
if mass_no!=0:
Its a isotope
else:
Its a normal element
except RuntimeError as e:
raise RuntimeError('Abundances are not defined properly in config', e.args)
Pyne
returns mass_no
zero whenever we pass elements in normal form(without mass_no
attached), something like this:
>>> nucname.anum('O')
0
>>> nucname.anum('Ni')
0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vg3095 try to figure out how to pass elements to pine - I'm sure that works.
@vg3095 I think that looks good - apart from the points I made - good job!! |
@vg3095 looks good @tardis-sn/tardis-core can someone cross check before I merge? |
This PR is for adding support to read
uniform isotope abundances
.For seeing sample output :
Notebook (Updated)
Config (Updated)
For Isotopes , you will need to mention mass-no. in every case.
For example
Ni
will not work ,Ni56
,Ni58
will work.It is because ,I think Tardis does not store
mass_no.
of stable isotope elements, so as of now , it supports this type of format(ElementMass_no
)