Add variable to specify ZAID-format #187

yardasol · 2023-01-18T22:49:52Z

Summary of changes

This PR addressed the bug described in #171. Specifically, this PR adds:

A new input variable, zaid_convention
Control flow to existing functions in SerpentDepcode that utilize zaid_convention to choose how to process nuclide ZAID codes.
Tests to cover these changes
Consistency changes in relevant example and test files.

Note to reviewers: The majority of the 273k added lines are in two files: tap_reference_mcnp.out and tap_reference_nndc.out, located at tests/serpent_data/. These are identical to serpent_data/tap_reference.out, except I have changed the ZAIDs for metastable nuclides to match what we would expect them to.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Required for Merging

Associated Issues and PRs

Issue: closes [Bug]: DepcodeSerpent.get_nuc_name() fails with MCNP-style metastable nuc-codes. #171

Associated Developers

Dev: @

Checklist for Reviewers

Reviewers should use this link to get to the
Review Checklist before they begin their review.

…ventions

abachma2

Hi @yardasol. Great work! I think this really adds some good flexibility to SaltProc to use any of these ZAID conventions. I have a few questions for you. I did not look at the two files in your note to reviewers, but can do that if necessary.

In your test suite, it looks like you test converting MCNP or NNDC conventions to Serpent convention. Do the functions only go in that direction? Or do they do Serpent to MCNP/NNDC as well?

abachma2 · 2023-01-19T21:51:15Z

saltproc/input_schema.json

-                                "pattern": "^(.\\/)*(.*)$"}
+                                "pattern": "^(.\\/)*(.*)$"},
+                            "zaid_convention": {
+                                "description": "ZAID naming convention for nuclide codes. 'serpent': The third digit in ZA for nuclides in isomeric states is 3 (e.g. 47310 for for Ag-110m). 'mcnp': ZA = Z*1000 + A + (300 + 100*m). where m is the mth isomeric state (e.g. 47510 for Ag-110m). 'nndc': Identical to 'mcnp', except Am242m1 is 95242 and Am242 is 95642",


I'm curious why you included NNDC in this, when it looks like your stuff just uses Serpent and OpenMC.

NNDC provides cross section libraries using their own ZAID convention for metastable nuclides. It is slightly different than the convention used by cross section libraries shipped with MCNP. Notice that this variable only appears in the depcode object if codename is serpent. This is because Serpent uses ZAID codes to get cross section data for nuclides. This variable has no effect on how OpenMC will handle it's XS libraries, as that is all internally consistent within OpenMC.

abachma2 · 2023-01-19T21:53:45Z

saltproc/serpent_depcode.py

+                if self.zaid_convention == 'mcnp' and nuc_code in (95242, 95642):
+                    if nuc_code == 95242:
+                        nuc_code = 95642
+                    else:
+                        nuc_code = 95242


Based on your documentation I thought 95642 was for the NNDC ZAIDs, not the OpenMC ones. This function makes it seem the opposite.

This has nothing to do with OpenMC. It is all about where the nuclide cross section data is sourced from, since their ZAIDs will have slightly different conventions for metastable isotopes. This is a bit confusing so I'm going to go through this step-by-step.

Recall that the standard nndc convention is identical to the mcnp convention, except in the case of Am-242 and Am-242m. The mcnp convention for ZAIDs of metastable isotopes is ZA = Z * 1000 + (A + 300 + 100*m). This means that Ag-110m has ZA = 47510, and Am-242m has ZA = 95642. In the nndc convention, for whatever reason, the ZA for Am-242m and Am242 are swapped, so ZA(Am242) = 95642, and ZA(Am242m) = 95242.

PyNE's nucname module contains functions for handling ZAID codes. One such function is nucname.mcnp_to_id, which converts the ZAID code in mcnp convention to PyNEs internal id format. However, during testing, I noticed that this function actually converts ZAID codes in nndc convention! That is, will convert 95242 to 952420001, and 95642 to 952420000, but convert everything else normally. So if we are using the mcnp convention, we'll need to input the wrong ZAID code to get the right PyNE id. Does that make sense?

That all mostly makes sense (I meant to say MCNP, but had OpenMC on my mind when I made the comment, apologies). Just to make sure I understand this correctly:

In MCNP:

Am242 = 95242

Am242m = 95642
In NNDC:

Am242 = 95642

Am242m = 95242.

Your function here says that if you are using MCNP and you give it 95242 (Am242 in MCNP), then it converts it to 95642 (Am242m in MCNP). If you give it 95642 (Am242m in MCNP), then it converts it to 95242 (Am242). So that gets switched. This assumes that you are providing NNDC nomenclature and trying to use MCNP. What if you are using MCNP nomenclature already and don't need to switch it?

You have the first right.

This assumes that you are providing NNDC nomenclature and trying to use MCNP. What if you are using MCNP nomenclature already and don't need to switch it?

The reason we do the swap is because PyNE assumes an nndc format for nucname.mcnp_to_id and other mcnp-related functions. That is, there is a discrepancy between our understanding of the mncp convention and what PyNE's understanding of the mcnp convention. PyNE's mcnp convention is equivalent to our nndc convention. So if we want to use our mcnp convention, we have to swap the ZAIDs for Am242 and Am242m that we input into the mcnp-related functions in PyNE to get the correct outputs (ZAIDs and nuclide names).

That makes sense. I'm making sure that there is no situation in which these ZAIDs would be switched when they don't have to be.

saltproc/serpent_depcode.py

abachma2 · 2023-01-19T21:58:47Z

tests/unit_tests/test_serpent_depcode.py

+    assert nuc_code_map[380880] == '38088.82c'
+    assert nuc_code_map[962400] == '96240.82c'
+    assert nuc_code_map[952421] == '95642.82c'
+    assert nuc_code_map[952420] == '95242.82c'
+    assert nuc_code_map[471101] == '47510.82c'


This seems to imply a material temperature. Based on the later assertions, I don't think this always happens. Does it just happen with these nuclides? What causes this? Can the user change the temperature?

I'm not sure what you're question is. Can you be more specific?

The .82c ending to these ZAIDs indicates a temperature of the material, doesn't it? If it doesn't, then what does it mean? The assertions you test later in this test don't have the ending, so it clearly isn't applied to all ZAIDs.

The .82c ending to these ZAIDs indicates a temperature of the material, doesn't it?

Correct.

The assertions you test later in this test don't have the ending, so it clearly isn't applied to all ZAIDs.

Some nuclides are decay-only, i.e. they don't have cross-section data.

Where exactly does the .82c come from, are you specifying a temperature somewhere? Is that a default? How will a user know what the material temperature is?

But within your code, where does the .82c come from? Where does that get set? Does the user define it? Will is always be .82c or can the user change that by specifying a temperature?

For the specific tests I added, these values are pulled from the .out files in tests/serpent_data/. When actually running SaltProc, Serpent creates this file. You can read about it here.

User's specify their material temperatures entirely in their Serpent input files based on the data library they are using.

So when a user is running SaltProc, they can specify the material temperatures, then SaltProc create the serpent output file as needed. Correct? And there is documentation on how a user specifies the material temperature? I want to make sure that SaltProc/Serpent will get and use cross section data for the correct temperature that the user wants.

So when a user is running SaltProc, they can specify the material temperatures, then SaltProc create the serpent output file as needed. Correct?

Yes, but all the temperature specification happens in the Serpent material and input files.

And there is documentation on how a user specifies the material temperature? I want to make sure that SaltProc/Serpent will get and use cross section data for the correct temperature that the user wants.

It's out of scope for SaltProc to tell people how to get cross section libraries or set up their input file past what format SaltProc expects, as those steps are different for each depletion code. I do have a guide in progress showing people how to structure the Serpent template input file here.

yardasol · 2023-01-20T18:50:34Z

In your test suite, it looks like you test converting MCNP or NNDC conventions to Serpent convention. Do the functions only go in that direction? Or do they do Serpent to MCNP/NNDC as well?

I think you are confused by the name of the function map_nuclide_code_zam_to_serpent. I'll change this so it's less confusing.

abachma2 · 2023-01-20T20:49:10Z

In your test suite, it looks like you test converting MCNP or NNDC conventions to Serpent convention. Do the functions only go in that direction? Or do they do Serpent to MCNP/NNDC as well?

I think you are confused by the name of the function map_nuclide_code_zam_to_serpent. I'll change this so it's less confusing.

The function name wasn't what was confusing (unless me statement about what is happening wrong). I wanted to make sure you didn't have functions in the PR that mapped to MCNP, as I didn't see any tests for that.

yardasol · 2023-01-24T18:52:30Z

The function name wasn't what was confusing (unless me statement about what is happening wrong). I wanted to make sure you didn't have functions in the PR that mapped to MCNP, as I didn't see any tests for that.

There are no such functions. If we ever add functionality to couple saltproc to MCNP, then we will.

yardasol · 2023-01-24T18:53:22Z

@samgdotson @LukeSeifert bump

abachma2 · 2023-01-24T21:12:07Z

Thanks for addressing my questions @yardasol. I approve the PR. I won't merge for a few days in case @samgdotson or @LukeSeifert want to look at this.

LukeSeifert

Looks good overall, though I'm not super familiar with the different ZAID formats. I had a few small comments/questions on some things.

saltproc/input_schema.json

LukeSeifert · 2023-01-24T21:07:21Z

saltproc/serpent_depcode.py

+            nuc_code = int(nuc_code.split('.')[0])
+            if self.zaid_convention == 'serpent':
+                nuc_code = pyname.zzzaaa_to_id(nuc_code)
+            if self.zaid_convention in ('mcnp', 'nndc'):


Consider changing to elif here, then you could add an else statement to make sure self.zaid_convention is an expected value.

LukeSeifert · 2023-01-24T21:19:23Z

tests/unit_tests/test_serpent_depcode.py

+    serpent_depcode.zaid_convention = 'mcnp'
+    assert serpent_depcode.convert_nuclide_code_to_name('92235.82c') == 'U235'
+    assert serpent_depcode.convert_nuclide_code_to_name('38088.82c') == 'Sr88'
+    assert serpent_depcode.convert_nuclide_code_to_name('95642.82c') == 'Am242m1'
+    assert serpent_depcode.convert_nuclide_code_to_name('95242.82c') == 'Am242'
+    assert serpent_depcode.convert_nuclide_code_to_name('61548.82c') == 'Pm148m1'
+    assert serpent_depcode.convert_nuclide_code_to_name('20060') == 'He6'
+    assert serpent_depcode.convert_nuclide_code_to_name('110241') == 'Na24m1'
+    assert serpent_depcode.convert_nuclide_code_to_name('170381') == 'Cl38m1'
+    assert serpent_depcode.convert_nuclide_code_to_name('310741') == 'Ga74m1'
+    assert serpent_depcode.convert_nuclide_code_to_name('290702') == 'Cu70m2'
+    assert serpent_depcode.convert_nuclide_code_to_name('250621') == 'Mn62m1'
+    assert serpent_depcode.convert_nuclide_code_to_name('300732') == 'Zn73m2'
+    assert serpent_depcode.convert_nuclide_code_to_name('370981') == 'Rb98m1'
+    assert serpent_depcode.convert_nuclide_code_to_name('390972') == 'Y97m2'
+    assert serpent_depcode.convert_nuclide_code_to_name('491142') == 'In114m2'
+
+    serpent_depcode.zaid_convention = 'nndc'
+    assert serpent_depcode.convert_nuclide_code_to_name('92235.82c') == 'U235'
+    assert serpent_depcode.convert_nuclide_code_to_name('38088.82c') == 'Sr88'
+    assert serpent_depcode.convert_nuclide_code_to_name('95242.82c') == 'Am242m1'
+    assert serpent_depcode.convert_nuclide_code_to_name('95642.82c') == 'Am242'
+    assert serpent_depcode.convert_nuclide_code_to_name('61548.82c') == 'Pm148m1'
+    assert serpent_depcode.convert_nuclide_code_to_name('20060') == 'He6'
+    assert serpent_depcode.convert_nuclide_code_to_name('110241') == 'Na24m1'
+    assert serpent_depcode.convert_nuclide_code_to_name('170381') == 'Cl38m1'
+    assert serpent_depcode.convert_nuclide_code_to_name('310741') == 'Ga74m1'
+    assert serpent_depcode.convert_nuclide_code_to_name('290702') == 'Cu70m2'
+    assert serpent_depcode.convert_nuclide_code_to_name('250621') == 'Mn62m1'
+    assert serpent_depcode.convert_nuclide_code_to_name('300732') == 'Zn73m2'
+    assert serpent_depcode.convert_nuclide_code_to_name('370981') == 'Rb98m1'
+    assert serpent_depcode.convert_nuclide_code_to_name('390972') == 'Y97m2'
+    assert serpent_depcode.convert_nuclide_code_to_name('491142') == 'In114m2'


This could be cleaner if you created a three lists of nuclide_codes, serpent_names, mcnp_names, and nndc_names then iterated through them. It would also be a bit easier to see the differences, but since this is a test it probably isn't necessary.

saltproc/input_schema.json

samgdotson

Looks fine to me, although I'm confused why there isn't a corresponding openmc_depcode.py file. Is that coming in a separate PR or...?

yardasol · 2023-01-26T22:30:01Z

Looks fine to me, although I'm confused why there isn't a corresponding openmc_depcode.py file. Is that coming in a separate PR or...?

Do you mean changes is the openmc_depcode.py file? OpenMC uses nuclide names (Am242) instead of ZAIDs, so this feature isn't relevant to the OpenMC side of things.

yardasol · 2023-01-27T18:45:25Z

@samgdotson is this good to merge?

yardasol · 2023-01-30T16:18:35Z

@samgdotson bump

…-zaid-conventions Add variable to specify ZAID-format 1af46aa

…ible-zaid-conventions Add variable to specify ZAID-format 1af46aa

yardasol added 6 commits January 18, 2023 15:29

handle differente zaid conventions in SerpentDepcode

2777bcc

update docpages and release notes

e90fe06

add nndc convention

2c5d14b

add tests

fb05f57

update example inputfiles

0d4b94b

update releasenotes

710516b

yardasol marked this pull request as ready for review January 19, 2023 20:59

yardasol added 2 commits January 19, 2023 15:02

Merge remote-tracking branch 'upstream/master' into flexible-zaid-con…

62d423b

…ventions

fix description of zaid_convention in the docpages

95de3cc

yardasol requested review from abachma2, samgdotson and LukeSeifert January 19, 2023 21:11

abachma2 reviewed Jan 19, 2023

View reviewed changes

yardasol mentioned this pull request Jan 20, 2023

[Bug]: Fix JSON Schema validation and add test coverage for all inputs #184

Open

yardasol requested a review from abachma2 January 24, 2023 18:52

abachma2 approved these changes Jan 24, 2023

View reviewed changes

LukeSeifert reviewed Jan 24, 2023

View reviewed changes

samgdotson approved these changes Jan 26, 2023

View reviewed changes

samgdotson merged commit 1af46aa into arfc:master Jan 30, 2023

github-actions bot pushed a commit that referenced this pull request Jan 30, 2023

update docs at commit: Merge pull request #187 from yardasol/flexible…

9da1bd5

…-zaid-conventions Add variable to specify ZAID-format 1af46aa

github-actions bot pushed a commit to yardasol/saltproc that referenced this pull request Jan 30, 2023

update docs at commit: Merge pull request arfc#187 from yardasol/flex…

6082559

…ible-zaid-conventions Add variable to specify ZAID-format 1af46aa

github-actions bot pushed a commit to khurrumsaleem/saltproc that referenced this pull request Jan 31, 2023

update docs at commit: Merge pull request arfc#187 from yardasol/flex…

faae8ae

…ible-zaid-conventions Add variable to specify ZAID-format 1af46aa

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add variable to specify ZAID-format #187

Add variable to specify ZAID-format #187

yardasol commented Jan 18, 2023 •

edited

Loading

abachma2 left a comment

abachma2 Jan 19, 2023

yardasol Jan 20, 2023 •

edited

Loading

abachma2 Jan 19, 2023

yardasol Jan 20, 2023

abachma2 Jan 20, 2023

yardasol Jan 20, 2023

abachma2 Jan 20, 2023

abachma2 Jan 19, 2023

yardasol Jan 20, 2023

abachma2 Jan 20, 2023

yardasol Jan 24, 2023

abachma2 Jan 24, 2023

abachma2 Jan 24, 2023

yardasol Jan 24, 2023

yardasol Jan 24, 2023

abachma2 Jan 24, 2023

yardasol Jan 25, 2023

yardasol commented Jan 20, 2023

abachma2 commented Jan 20, 2023

yardasol commented Jan 24, 2023

yardasol commented Jan 24, 2023

abachma2 commented Jan 24, 2023

LukeSeifert left a comment

LukeSeifert Jan 24, 2023

LukeSeifert Jan 24, 2023

samgdotson left a comment

yardasol commented Jan 26, 2023

yardasol commented Jan 27, 2023

yardasol commented Jan 30, 2023

Add variable to specify ZAID-format #187

Add variable to specify ZAID-format #187

Conversation

yardasol commented Jan 18, 2023 • edited Loading

Summary of changes

Types of changes

Required for Merging

Associated Issues and PRs

Associated Developers

Checklist for Reviewers

abachma2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yardasol Jan 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yardasol commented Jan 20, 2023

abachma2 commented Jan 20, 2023

yardasol commented Jan 24, 2023

yardasol commented Jan 24, 2023

abachma2 commented Jan 24, 2023

LukeSeifert left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samgdotson left a comment

Choose a reason for hiding this comment

yardasol commented Jan 26, 2023

yardasol commented Jan 27, 2023

yardasol commented Jan 30, 2023

yardasol commented Jan 18, 2023 •

edited

Loading

yardasol Jan 20, 2023 •

edited

Loading