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

Add information for closed fuel cycles #152

Merged
merged 104 commits into from
Nov 1, 2023
Merged

Conversation

abachma2
Copy link
Contributor

@abachma2 abachma2 commented Oct 9, 2023

This PR became much bigger than intended, so I apologize in advance for that. Many files have been added or modified, but hopefully this can all be broken up as follows:
(1) update the input/haleu/README.md with a more complete description of the different scenarios
(1) update the notebook for the once-through scenarios (input/haleu/analysis/once_through_analysis.ipynb)-- these scenarios were re-run because of updates to the advanced reactor deployment scheme
(1) new notebook (input/haleu/analysis/recycle_analysis.ipynb) for analysis of the closed fuel cycle scenarios
(2) Updates to the once-through scenario input files to have the correct path for my ANL desktop, and to have a separate enrichment facility for MMR fuel -- the separate facility allows the same commodity name to be used across all scenarios without having to duplicate files and prototype definitions

  • input/haleu/inputs/scenario2.xml - input/haleu/inputs/scenario13.xml
    (2) Rename once-through scenario input files to have scenario name
  • see file names in previous bullet
    (2) Update input/haleu/run_multiple_cyclus_inputs.sh to reflect file name changes
    (3) Add a notebook (input/haleu/inputs/openmcyclus-files/cross-sections.ipynb) to show how I generated cross section data for the Xe-100, VOYGR, and SFR
    (3) Add input/haleu/inputs/openmcyclus-files/htgr-mr-full-core.inp_mdx0.m, which is the cross section data from Serpent. This file is processed in the notebook mentioned above.
    (3) Add chain files and materials files for the Xe-100, SFR, and VOYGR, located in input/haleu/inputs/openmcyclus-files/reactor-name/. These files are read in by OpenMC
  • materials.xml and chain_casl_pwr.xml/chain_casl_sfr.xml in each of the directories.
    (4) Input files for Scenarios 14-19, with descriptions in the input/haleu/README
  • input/haleu/inputs/scenario14.xml - input/haleu/inputs/scenario19.xml
    (4) Update/add files to define the different reactor prototypes, in input/haleu/inputs/united_states/reactors/
    (5) Update and add recipes for the advanced reactors -- the spent fuel recipes for MOX and the SFR are arbitrary, the recipe name needs to exist but the composition of the spent fuel is determined by the OpenMC depletion for the spent MOX and SFR fuel.
  • files in input/haleu/inputs/united_states/recipes/
    (5) Remove a file for a DeployManagerInst Institution that is no longer needed
  • input/haleu/inputs/united_states/buildtimes/UNITED_STATES_OF_AMERICA/deploymanagerinst.xml
    (6) add the get_pris_powers function back to scripts/create_AR_DeployInst.py to reproduce the procedure for create the transition DeployInsts. I previously removed this function, thinking it was not needed, but it is
    (6) update scripts/dataframe_analysis.py to reflect updates in pandas, add function for getting all material transactions to a specified prototype name
    (6) Add function to scripts/transition_metrics.py to add the prototype name associated with each SenderId.

Feel free to break up the PR as needed, sorry again for the PR creep. Let me know if you have any questions, like if the files that pertain to each item aren't clear.

This PR contains the final code changes for Amanda Bachmann's dissertaion

New facilities are added and commodity names are changed to
ensure that all reactors receive different commodity names. This
is required to ensure that each advanced reactor (AR) recieve
the recipe it needs so that (eventually) depletion will be
performed on an accurate recipe
MMR Mox facilities are removed because the MMR never gets
refueled, so there is no reason to produce MOX to go into the
MMRs
Change the MMR to not accept MOX, only UOX. Spent UOX from LWRs
is no longer sent to separations to become MOX
remove DryStorage, chagne Xe100 and VOYGR to OpenMCyclus files,
change LWREnrichment max enrichment level to 5%, remove U
separation, correct spent MMR commodity name, add DeployInst
to deploy the Separations facility in 2020.
@samgdotson
Copy link
Contributor

Without looking at the PR yet, I can say that it does bother me that, from your description, it sounds like you have hard-coded paths to get the code to work on a specific computer, which I don't love.

@abachma2
Copy link
Contributor Author

abachma2 commented Oct 9, 2023

I figured out a way to make them all relative paths, but that means that all the input files must be run from input/haleu. I made a note about this in the README for the directory.

Copy link
Contributor

@samgdotson samgdotson left a comment

Choose a reason for hiding this comment

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

Good job on this PR, clearly a lot of work went into it and I don't want my comments to discourage you. Most of them are nitpicky so we can discuss the choices you made where appropriate.

Comments on your analysis notebooks.
Good job using colors that are color blind friendly (i think) and attempting patterns that in theory distinguishes each curve.

  1. Many of your figures are missing titles and axis labels. (e.g., annual mass recycled?)
  2. Gotta say, I really don't love the "scenario x" pattern. It makes the legends neater, sure, but the plots don't stand for themselves, which make it very difficult to quickly understand your results. Possibly remedied by plot titles.
  3. If these are plots you will use in your thesis, I recommend plotting using PGF plots (which makes them vector graphics rather than static PNG files).
  4. If thesis plots, make fonts much bigger.
  5. Also the plots could be... wider. Especially the spiky ones. It's hard to discern much of a pattern in the noise.
  6. Make sure your plots have a white background -- many of them do, but not all.

@@ -10,8 +10,8 @@ Inputs:
This directory holds all of the input files for this project.

The input files in the repository are the ``.xml`` input files for
|Cyclus|. The input files are titled by the type of advanced
reactors to be deployed and the demand growth of the scenario.
|Cyclus|. The input files are titled by scenario number, with
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a good idea to come up with a naming convention, but I recommend making the names more informative than "scenarioX.xml". I will have to reference the table below in order to figure out which table is which. ALSO, what if you change the files and the results change? You'll have to update everything somehow which challenges reproducibility and code maintenance.

Just as a quick suggestion, what about something like:

FC{OT or C}_AR0{0,1,2,3}_GPCT{0, 1, NA}_SC{#}_V{00X}.xml

(fuel cycle) (advanced reactors) (growth percent) (scenario number) (version)

For scenario 2 this would look like

FCC_AR01_GPCT00_SC02_V001.xml

It might be a bit more cumbersome, but once users become familiar with this naming convention, it will help them (and also possibly you) figure out what each input file is supposed to run. The table in this file is good and useful, but you need a stronger naming convention for your files. Also include a description of your naming convention -- and stick to it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This feels less intuitive to me, and your example for how it works is wrong, as Scenario 2 is a once-through fuel cycle. I feel like your convention would still require referencing back to a table of which advanced reactor number corresponds with which advanced reactor. I'm also not sure why you would include a version number in the file name if it's all version controlled.

I had previously used a convention of reactor1_..._reactor_N_growth.xml, which worked well for just the once-through scenarios. Adding in the closed fuel cycles, I felt that the file names would get too cumbersome and inconsistent (especially given that Scenarios 14 and 15 only differ by the material being reprocessed) and I felt like using the scenario numbers would be the easiest and cleanest. I understand that this may require users to reference the table in the README, but that's why I made sure to include the table as the input file names aren't descriptive without it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Apologies for the error in my example. The purpose of having a version number is in case you had multiple updated files. I don't think there's a "release" associated with this repository. It can be hard to reference a specific file version without a release tag.

Sure, it's cumbersome, but could be useful if there were many more input files than currently exist. You don't have to use it. However, I don't think users should be required to reference a readme in order to understand approximately what the file contains.

Copy link
Contributor

Choose a reason for hiding this comment

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

As a comment, final analyses should be in their own folder. It should not be a subfolder of "input." Finding and referencing these notebooks is not intuitive.

My imagined internal dialogue of a future reviewer:

"Ah where are her results?"

"The... input folder? Seems a bit recursive, no?"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This repository contains files related to modeling a variety of transition scenarios. The top-mostinput directory is for the files for modeling all of the transitions. I did not set up the general structure of this repo. I understand that this can be confusing, which is why there is a README in the top-level directory and in the input/haleu directory.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Ok I just looked through the README in the input/haleu directory. Sa,. the transition-scenarios repo contains transitions beyond just amanda's work and contains the work from other students as well.

I like the README in the haleu directory and I think it helps describe what is in the haleu directory. Alongside haleu, there are other transition scenarios in the input folder. Sam, does this help clarify amanda's structure? Do you maybe have a different suggestion that could help clarify this structure?

Amanda, I see that you clearly describe the input files and the analysis files. The analysis notebooks are contained in the input/haleu/analysis folder, and then inputs for your transition are in input/haleu/inputs folder. Is that right? So where are the output files going to be generated if somebody runs these, and where do the analysis notebooks expect the outputs to be? It it would be helpful to mention that in the readme.

Copy link
Contributor

Choose a reason for hiding this comment

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

This issue could resolve itself if the top-level inputs folder was renamed to scenarios.

Copy link
Member

Choose a reason for hiding this comment

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

I added the people from this conversation to the PR, but, for posterity's sake, refer to #153 (which tries to rectify these comments).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I place the output databases in inputs/haleu/outputs. That directory isn't in this repo because those files get generated by running the inputs. The run_multiple_inputs and the analysis notebooks also assume this, hit it can easily be updated as desired. I can add this information into the readme for the haleu directory though.

input/haleu/inputs/scenario13.xml Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

See, this is exactly what I'm talking about with the naming convention. I actually like MMR_nogrowth better.

Copy link
Contributor

Choose a reason for hiding this comment

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

You would just need to update your table to reflect the more descriptive file names. Adding a column shouldn't be that bad.

@@ -1,5 +1,5 @@
for scenario in mmr xe100 xe100_mmr mmr_voygr xe100_voygr xe100_mmr_voygr
for scenario in 2 3 4 5 6 7 8 9 10 11 12 13
Copy link
Contributor

Choose a reason for hiding this comment

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

okay, I see why you wanted to have this relatively simple naming convention. I think my suggestion still works, since the scenario number is contained. Since you're using a bash script, you can be a LOT more creative with how you're accessing files.

E.g. save the files in your input folder as a list variable, then loop through each item in the list variable, rather than a numerical range.

Copy link
Contributor

Choose a reason for hiding this comment

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

Idk I'd also want a way for users to run specific scenarios interactively? I think you could do this. But it's not required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Users can always run a single input file by running cyclus -i inputs/file names -o outputs/output_database.sqlite. This script was a way for me to easy run them in series so that I didn't have to keep typing that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, but what about a specific set of scenarios (e.g., 1,4,7)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then they can run those in series from the command line or adjust this file to include only the scenarios they want to run.

scripts/create_AR_DeployInst.py Show resolved Hide resolved
scripts/dataframe_analysis.py Outdated Show resolved Hide resolved
scripts/transition_metrics.py Show resolved Hide resolved
@abachma2
Copy link
Contributor Author

abachma2 commented Oct 9, 2023

Thanks for the review @samgdotson. Did you get through all of the files? Are there any that need to/should be reviewed by another set of eyes?

@abachma2
Copy link
Contributor Author

abachma2 commented Oct 9, 2023

1. Many of your figures are missing titles and axis labels.  (e.g., annual mass recycled?)
2. Gotta say, I really don't love the "scenario x" pattern. It makes the legends neater, sure, but the plots don't stand for themselves, which make it very difficult to quickly understand your results. Possibly remedied by plot titles.
3. If these are plots you will use in your thesis, I recommend plotting using PGF plots (which makes them vector graphics rather than static PNG files).
4. If thesis plots, make fonts much bigger.
5. Also the plots could be... wider. Especially the spiky ones. It's hard to discern much of a pattern in the noise.
6. Make sure your plots have a white background -- many of them do, but not all.

Each of the figures are meant to go in my thesis. The general trends are more important that understanding what is happening in each single data point. This is also why annual averages are plotted for most of the results. All of the files should be saved as PDFs. I'll make sure they all have white backgrounds.

@abachma2 abachma2 requested a review from samgdotson October 11, 2023 19:28
@abachma2
Copy link
Contributor Author

After talking with @munkm about the input file naming convention, I reverted the file names back to what they previously were for the once-through fuel cycles and came up with a similar convention for the closed fuel cycles. I added this information to the directory README.

@abachma2
Copy link
Contributor Author

Any further comments/suggestions for this PR? @samgdotson @munkm?

@samgdotson
Copy link
Contributor

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants