mit-crpg / opendeplete

A depletion framework for OpenMC
MIT License
15 stars 5 forks source link

Refactor depletion chain data #29

Closed paulromano closed 7 years ago

paulromano commented 7 years ago

This pull request makes some major changes to the DepletionChain class and modest changes to Nuclide. I've combined the data from the Yield class into the Nuclide class (as part of the yield_data and yield_energies attribute). Thus, now the decay, reaction Q values, and yield data appears together in the same class. Together with this, the format of the chain.xml file has also changed:

There is now a make_depletion_chain function that will create a chain file given a list of ENDF decay files, FPY files, and incident neutron files (used to get reaction Q values). When I create a chain file from ENDF/B-VII.1 data, there are a few data issues that should be noted:

The full test suite passes for me. I need to do some more testing with a full chain file generated from ENDF/B-VII.1 data, but thought I'd submit this PR so that you can start reviewing.

cjosey commented 7 years ago

This is becoming a more complicated review than I anticipated. I'll have it done in an hour or two.

paulromano commented 7 years ago

I've confirmed that this capability works with JEFF data (decay/FPY library from JEFF 3.1.1, neutron library from JEFF 3.2). There are a few neutron files that are badly formatted (e.g., Gd-152 is ISO-8859-1 encoded which is against ENDF rules) but once they are fixed everything runs fine. There are actually fewer errors reported (no fission products that don't have decay data, no missing reaction products).

cjosey commented 7 years ago

So, last few notes, as this is close to merging.

  1. It is probably safe to remove ``chain_full.xml'' as it is formally obsolete.
  2. I am getting subtle numerical differences (17th digit sort of things) between my chain and the LFS one. This really doesn't matter and probably doesn't need to be fixed. As long as a user uses the same chain, then RNG sequences will be fine.
  3. The new download script is super nice.
  4. import sys in depletion_chain.py isn't needed.
paulromano commented 7 years ago

Thanks for pointing me to tqdm by the way, I love it. I'll get rid of the old full chain file. Do you want me to add in a chain file for JEFF too?

cjosey commented 7 years ago

I'm not sure on JEFF. See, I don't know if it would ever be used before the chains are modified again for capture branching ratios. I'll leave that choice up to you.

Also, before I forget again, I did email Kord asking about how he would handle the missing yield data.

I guess I would use yield data for the closest isotope A number and scale to conserve mass.

Ugly call!

I am fine leaving it as is for now, as I would really rather do some research on the topic to see what is done elsewhere.

paulromano commented 7 years ago

Good point, let's wait for branching ratios (I'll start cogitating on that one).

paulromano commented 7 years ago

@cjosey I believe that I've addressed all your actionable comments. Let me know if I'm still missing something.

cjosey commented 7 years ago

It all appears good. In fact, I was making one last check to ensure the yields were right and discovered that the old decay chain was much worse than it should've been. For example Xe135's U235 yield was the total yield (stable + metastable). So, I guess we can count this as a bugfix release.