search

arfc / transition-scenarios

A repository to hold transition scenarios with Cyclus.
Other
3 stars 12 forks source link

Add information for closed fuel cycles #152

Closed abachma2 closed 10 months ago

abachma2 commented 11 months ago

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

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

pep8speaks commented 11 months ago

Hello @abachma2! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 180:62: W291 trailing whitespace Line 205:60: W291 trailing whitespace

Line 234:80: E501 line too long (98 > 79 characters)

Comment last updated at 2023-10-25 16:45:00 UTC
samgdotson commented 11 months ago

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 commented 11 months ago

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.

abachma2 commented 11 months ago

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 commented 11 months ago 
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 commented 11 months ago

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 commented 10 months ago

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

samgdotson commented 10 months ago

lgtm