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

Update IUPAC 2013 Abundance Data #2586

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

quantaroo
Copy link

@quantaroo quantaroo commented Jun 30, 2023

Description

References: #2581, #2585, and #2423

Updated the IUPAC 2013 to reflect the better practice of averaging the isotope abundance intervals of column 9 when the interval is present. Otherwise, the best case value from column 6 is used.

Fixes # (issue)

Updated the values and description.

Checklist

  • I have performed a self-review of my own code
  • I have run clang-format on any C++ source files (if applicable)
  • I have followed the style guidelines for Python source files (if applicable)
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

Copy link
Contributor

@paulromano paulromano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for splitting up these PRs @quantaroo! Other than the line comments below, some test results will likely need to be updated in order to get them passing.

openmc/data/data.py Show resolved Hide resolved
Comment on lines 21 to 22
'N15': 0.0037950, 'O16': 0.9975700, 'O17': 0.0003835,
'O18': 0.0020450, 'F19': 1.0, 'Ne20': 0.9048,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm honestly not sure what to suggest for the 3-isotope interval cases. What is definitely not acceptable is to have the abundances sum to something other than 1. That leaves the following options:

  1. Slightly modify a single value so that they sum to unity
  2. Modify all values the same amount so that they sum to unity (will result in weird values)
  3. Stick with the "best measurements" for these cases

I lean toward option 1 but would definitely like to hear others thoughts.

@quantaroo
Copy link
Author

Hi @paulromano
There are some regression tests that will be fixed once the 3 isotope element issue and other tests are fixed and implemented. I will make the fix for the 3 isotope element issue once we have more consensus. I think the first option is perfectly fine given how small the discrepancy is. Probably put/take from the largest isotope fraction.

Here are the fixes I will be working on for the other tests:
test/unit_test/test_data_misc :: test_isotopes - I will need to update H1 and H2
test/unit_test/test_data_misc :: atomic_weight - Update C values update
test/unit_test/test_material :: test_borated_water - Update B10, B11, H1, and O16
test/unit_test/test_model :: pin_model_attributes - Update the initial values to fix the other test functions.

If you see any other test I missed, let me know.

@paulromano
Copy link
Contributor

@quantaroo I like the idea of putting/taking from the isotope with the largest fraction. Can you go ahead and implement that here along with updating test results? Thanks!

@quantaroo
Copy link
Author

quantaroo commented Jul 10, 2023

@paulromano I am waiting for @yrrepy on confirmation for the largest fraction. I am having issues finding the new density to update all the densities in the test in test_material::test_borated_water, in test_model::test_py_lib_attributes, and in test_model::test_deplete. I tried printing the output values out but in the test but the test doesn't print these values out. What are your suggestions? Any thoughts @gridley

@yrrepy
Copy link
Contributor

yrrepy commented Jul 12, 2023

Yesterday, I shared with Juris Meija, one of the authors of IUPAC-2013, our current plans, and he has recommended using the IUPAC delta-zero materials. See attachment.
RE IUPAC Isotopic Compositions Column 9.zip

This essentially comes back to, when an interval is present, using IUPAC-2013 col.6 values, except when the delta-zero material differs, which is the case for Li, B, Mg, Si, S, Cl.
I could not find the delta-zero material abundances on https://ciaaw.org/, but Jaccob did find them here:
https://pubs.usgs.gov/wri/wri014222/pdf/wri01-4222.pdf
https://www.degruyter.com/document/doi/10.1515/pac-2013-1023/html?lang=en

I suppose we could:

  • leave the abundances in OpenMC as is, as the differences of the above elements with col.6 are very small.

  • Or could follow Juris' delta-zero recommendation (column M in the attachment)
    IUPAC_abundances_PJY2.xlsx

  • Or use NuBase4 with largest fraction maximizing.

Thoughts?

I'm not a fan of IUPAC-2013, the intervals and messiness with selecting from different spaces. Hence the desire to easily default to IUPAC-2009 (NIST).

@paulromano
Copy link
Contributor

Ugh, this really is a mess. The thing I worry about with the delta zero materials is that it's hard to even call it "IUPAC 2013" data at that point since those don't show up in the IUPAC 2013 publication. My inclination would be to either use NUBASE 2020 with the small adjustments discussed or just leaving the abundances as is.

@yrrepy
Copy link
Contributor

yrrepy commented Jul 17, 2023

The delta-zero not being in IUPAC 2013 is odd.
Are there any other opinions/leanings out there? Can we ask NUBASE/AME people?

@paulromano
Copy link
Contributor

@yrrepy I can ask the lead author on the NUBASE paper who is a fellow scientist at Argonne.

@quantaroo
Copy link
Author

@paulromano Any updates from your fellow scientist at Argonne?

@paulromano
Copy link
Contributor

@quantaroo Yes, sorry for the delay. I did finally get a chance to consult with him. He suggested one of the options we had above, which is to arbitrarily set the abundance for one of the isotopes to (100 - sum of others).

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