search

arfc / saltproc

Online reprocessing for molten salt reactors
Other
19 stars 16 forks source link

[Bug]: Failing Integration Tests #72

Closed yardasol closed 2 years ago

yardasol commented 2 years ago

Expected behavior The two integration test cases -- const_repr, and no_repro -- under saltproc/tests/integration_tests should both pass

Describe the bug const_repr Test fails due to input error

Input error in parameter "mat" on line 3 in file "/home/ooblack/projects/saltproc/data/saltproc_mat":

Nuclide 47310.09c not found in data libraries

Note that the specified nuclide has Z=47. This is Ag (silver). No known isotope of silver with N=310 exists.

no_repro Test fails due to pointer error

Fatal error in function GetText:

Invalid pointer: pointer less than valid pointer or grader than the ascii data size

Simulation aborted.

To Reproduce Steps to reproduce the behavior:

  1. If you've installed SaltProc in a conda or other virtual environment, activate that environment
  2. cd saltproc
  3. pytest saltproc/tests/integration_tests

Screenshots In lieu of screenshots, below are gists to the two error stacks. const_repr no_repro

System Specifications:

How can this issue be closed? This issue can be closed when the integration tests are passing.

yardasol commented 2 years ago

Updates

no_repro

I recompiled Serpent in debug mode and ran the serpent input for the no_repro integration test in gdb:

INPUT=$HOME/projects/saltproc/saltproc/tests/integration_tests/no_repro/int_test
gdb --args sss2-debug -omp 1 $INPUT

This is the output I get doing some thread tracing. The TL;DR is that something is going wrong when serpent is parsing the geometry file provided (tests/test_geo.inp).

const_repr

I did a similar process for the const_repr case:

INPUT=$HOME/projects/saltproc/data/saltproc_serpent
gdb --args sss2-debug -omp 1 $INPUT

However, now that we are using debug mode, I get the following error message:

Fatal error in function ReadFissionYields:

Value 8.019800E+04 of parameter "ZA" above upper limit 8.000000E+04 

NOTE: This value check was performed because the code was compiled in the
      debug mode and it may or may not be an indication of an actual problem.
      The debugger mode can be switched off by recompiling the source code
      without the -DDEBUG option (see Makefile).

My best guess is that this check is trying to make sure we aren't using any fission yields that are unrealistic/unlikely, but my expertise with that is limited so I can't say for sure. @munkm any ideas?

Going back to the issue of the 47310.09c key, this is present in line 683 in data/saltproc_mat, which is generated when we run the integration test. This value is sandwiches within the 471xxx section, so I believe that the 3 is a typo and should be a 1 instead. Tracking down where these keys are being stored is the next step.

yardasol commented 2 years ago

Updates

const_repr

I tracked the source for generating the data/saltproc_mat file to examples/tap/mats/. The file of interest is either saltproc_prepr_comp.ini or saltproc_prepr_conp_endf.ini.

I changed the 3 to a 1 as discussed above, and reran serpent. I found the following further "typos" through this process:

after doing this, I got a different key error:

Input error in parameter "therm" on line 11 in file "../../../../examples/tap/mats/non_burnable_mats.ini":

S(a,b) data hzr05.32t not found in libraries

Clearly I am missing thermal scattering data in the cross section library I am using serpent with. How can this be?

Recall the cross section library I specified above, JEFF 3.1.2. Serpent does not come with JEFF 3.1.2, and on the JEFF website, I was unable to find an xsdir file for JEFF 3.1.2, so I had to construct my own. We should include/specify a cross section library that users can download for use in testing the code like OpenMC does to prevent them from having to go through the steps I did below

Luckily, the JEFF 3.1.2 library provided does come with .dir files for each individual nuclide. Here's what I did to make the .xsdir file:

  1. Write a few lines of bash code to process the .dir files into all into a single file, sss_jeff312.dir. Also, append the temperature on each .ACE file and put them all in the same directory. I wrote a script to do this.

  2. Edit the sss_jeff312.dir file to have a regular delimiting character using vim's recording feature

      1. remove double spaces with %s/ / /g
      1. add spaces to the beginning of each line utilizing the record feature, and then remove double spaces again. This step ensures all lines have one whitespace at the beginning of each line.
      1. remove spaces at the beginning of each line using the record feature again.

  3. Open sss_jeff312.dir as a dataframe in pandas

    import pandas as pd
df = pd.read_table('sss_jeff312.dir`, sep=' ', names=[1,2,3,4,5,6,7,8,9,10,11], index_col=False)
df = pd.sort_values(1)
df.to_csv('sss_jeff312.dir`, sep=' ', header=False, index=False)

    These steps put the filename keys in the right format. I wrote a script to do the however we also need atomic weight ratios at the beginning of the file.

  4. Repeat step 2 on the intermediate 600.dir file (I'm picking this one for simplicity, any of them should work) created by the script, and then remove the .06c from the end of each key.

  5. Open 600.dir as a dataframe in pandas

    import pandas as pd
df = pd.read_table('600.dir`, sep=' ', names=[1,2], index_col=False)
df = pd.sort_values(1)
df.to_csv('600.dir`, sep=' ', header=False, index=False)

  6. Combine the files with the correct headers:

    FILE=sss_jeff312.xsdir
cat "datapath=$XSDIR/jeff312/acedata" >> $FILE
cat "atomic weight ratios" >> $FILE
cat 600.dir >> $FILE
cat "directory" >> $FILE
cat sss_jeff312.dir >> $FILE

    I didn't include the thermal cross section data in this file, which is what I'll have to do now. I assume it will be a similar workflow to what I have described.

yardasol commented 2 years ago

Updates

const_repr

I repeated the workflow above for the JEFF 3.1.2 thermal scattering data, which I recorded in this script. As I suspected, Serpent can now read its required cross sections. Unfortunately, I'm running into a new issue:

Input error:

File:

"/home/ooblack/projects/cross-section-libraries/jeff312/acedata/HZr-800.ACE"

must be converted from DOS to UNIX format

Luckily, we can easily do this using vim:

vim '+set ff=unix' '+x' file.txt

I do this for the following files:

That fixes the issue. Now we have another issue pop up:

Fatal error in function CheckNuclideData:

Duplicate found ZAI 471100 id = 09c T = 900.0T

Simulation aborted.

I found this page on the serpent forums which I'll read to try to fix it but I think that's enough debugging for one day.

munkm commented 2 years ago

@LukeSeifert am I remembering correctly that when you were using Serpent you ran into issues with it producing nuclides that didn't exist in the library it is distributed with? What was your solution?

munkm commented 2 years ago

My best guess is that this check is trying to make sure we aren't using any fission yields that are unrealistic/unlikely, but my expertise with that is limited so I can't say for sure. @munkm any ideas?

I would trust that our cross section libraries are correct, and there's an error elsewhere.

LukeSeifert commented 2 years ago

I was getting a similar error using JEFF libraries, switching to ENDF/B-7 fixed the issue for me. I believe the issue is that the SaltProc material uses a generated isotope list (msbr_saltproc_prepr_comp.ini) generated using the ENDF/B-7 libraries. Andrei suggested I could generate a new isotope list with JEFF using this or switch to ENDF, and I decided to switch since I just wanted to get SaltProc running.

yardasol commented 2 years ago

Update

I set up the jeff312_all_temps library from our group box folder on my machine and ran the integration tests.

no_repr

I get the same error as before.

cont_repr

I get the same error of missing materials that are in actuality due to typos in the xsdata file. After fixing the typos (see this comment and the one immediately below it), I get the following error:

Fatal error in function ReadACEFile:

Unable to find isotope 47110.09c in file /home/ooblack/projects/cross-section-libraries/jeff312_all_temps/ACEs_900K/Ag110M.ACE

Simulation aborted.

This file does exist at the specified path on my computer, so I don't know why serpent says it can't find it.

I ran this test case within a gdb session, set a breakpoint at readacefile.c:251, and executed the code. Perfoming a backtrace reveals that one of the variables within this function has the value HZ1 = "47310.09c... (where ... represents many more characters that don't need to be posted here). I checked the ACEs_900K/Ag110M.ACE file and confirmed that this character set exists there, at the very top of the file:

 47310.09c  108.962000  7.7556E-08   02/29/12

I suspect that the rest of the ACE files whose corresponding entry in xsdata had a typo may also be effected by this. @gwenchee note that the jeff312_all_temps archive on the box is bugged!!

This does not appear to be an issue for the ACE files I downloaded from OECD.

EDIT I just realized that the typos in the JEFF library on our group box and the typos in the material .ini files in the master branch of this repo match, so running the version of saltproc on the current master branch with the JEFF library from the box should allow the const_repr case to run.

I checked out the master branch, deleted the data folder so that it would regenerate when running pytest and ran pytest saltproc/tests/integration_tests/const_repr. The simulation proceeded further than I had ever seen it go before! Unfortunately, it still failed with the following error:

Processing cross sections and ENDF reaction laws...

Nuclide   1001.09c -- hydrogen at 900K (H-1)
Segmentation fault (core dumped)

This is very opaque. I'll do some more debugging on this probably tomorrow to see if I can uncover what's going on

yardasol commented 2 years ago

Update

For completeness:

const_repr

I ran this case with jeff312_all_temps in gdb. When I hit the segfault, I backtraced. Here is the output of that. My guess is that the code is trying to access an index in the array RDB that does not exist. I'm not sure why it would be passing a bad index to the array in the first place. I'll be making a serpent forum account and creating a few posts there to see if I can get any help debugging this.

yardasol commented 2 years ago

Update

const_repr

I made this post on the serpent forum asking, and shortly thereafter received a reply from Ana Jambrina pointing me to this patch. I applied the patch to the source code, recompiled, and ran the cons_repr test case with the jeff312_all_temps library, and it passed! (hooray!).

My guess is that these integration tests were written using a previous version of Serpent2 (2.1.30 or 2.1.31 I believe). Updates to Serpent2 included in 2.1.32 may have broken them, but with the aformentioned patch, the const_repr case is now working!

no_repro

As a refresher, the no_repro test case is failing when assigning boundary conditions to cells. At first I thought it was an issue with the test_geo.inp geometry input file, but my experience with fixing the const_repr case give me cause to scrutinize the internals of Serpent a bit more closely.

In the processsymmetries.c file, there is an if-else if -else statement that sets the boundary condition specified by the user. There is a single if-statement preceding the main if-else if sequence, which I believe is what is causing our problem.

See my post on the serpent forum for a more complete description. The TL;DR is that I added a else to the front of the 2nd if-statement, making the whole thing one big if-else if-else statement. After recompiling serpent, I ran the test case and it's now passing!!

Update 11/30: Ana pointed out that the syntax for the set usym card in the test_geo.inp input file uses the old Serpent 1 syntax. I changed the card to the new Serpent 2 syntax and ran the test case without the modification to the if-statement in the source code. The case ran successfully and passed the test! I will be making an issue that verifies all the test input files are using the correct Serpent 2 syntax, and to require updating the syntax if they are outdated.

yardasol commented 2 years ago

All the mysteries of the failing integration tests have been more or less resolved, so I'm closing this.