Closed abachma2 closed 2 years ago
Hello @abachma2! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:
There are 116 files to view here. How many of these are generated? If the majority of them are all manual, consider using @gwenchee's strategy of splitting up groups of changes for different reviewers.
EDIT: I reread the summary. Ignore this comment.
I think I have made all of the requested changes. I removed the LWR input files from the repo because they can be generated using the scripts available in the repo. This was something I had been debated if it was necessary, and I think it is to help keep PRs manageable. I also removed the input/haleu/analysis/scenario_data_analysis.ipynb
because it has been renamed to initial_data_analysis.ipynb
but git was still tracking it. I will have another PR later for my data analysis, which will include more changes to scripts/transition_metrics.py
and scripts/tests/test_transition_metrics.py
as needed.
To answer your questions:
scripts/create_input.py
, but I manually created the advanced reactor input files because there were not as many to create. .xml
files. I added the scripts I used to help create the actual recipe definitions. input/haleu/inputs/united_states/recipes/depletion_txt.py
uses the depleted fuel compositions from serpent and puts them into a text file. input/haleu/inputs/united_states/recipes/txt_xml.py
then reads in the text file and saves the contents to an xml file with the required formatting. The contents of the outout file is then copied into the respective .xml
file to be included in the recipes for that reactor. Let me know if I missed something.
I made the new requested changes, but I'm not sure what you are talking about that needs to be fixed in input/haleu/README.rst
.
I made the new requested changes, but I'm not sure what you are talking about that needs to be fixed in
input/haleu/README.rst
.
Right now, there is a sentence in input/haleu/README.rst
that reads:
Definitions for each advanced reactor and the corresponding recipes
is in ``inputs/united_states/reactors/'' and ``inputs/united_states/recipes'',
The subject ("Definitions for each advanced reactor and the corresponding recipes") is plural but the verb ("is") is singular. The case of the subject and verb should match, so the verb should be "are":
Definitions for each advanced reactor and the corresponding recipes
are in ``inputs/united_states/reactors/'' and ``inputs/united_states/recipes'',
Additionally, I noticed that at the end of the file path specifications, you use two single commas ('') instead of the baccent character (``). I'll make the inline comments for ya to commit
Don't forget to address the style comments from @pep8speaks !
Made those revisions and made sure to make the pep8 fixes.
Great work. Thanks for sticking with me on my comments
I have made a series of new input files to run simulations with multiple advanced reactors. Some of the changes include moving each of the advanced reactors to separate xml files to be read in as needed in each scenario and separate xml files for the recipes corresponding to each advanced reactor. I also created cyclus inputs and xml files to model teh NuScale VOYGR reactor alongside the X-energy Xe-100 and the USNC MMR.
I also modified the existing xml files for the LWRs by adding their current license expiration dates to the database that is read in and created to generate all of the input files for the reactors. The deployment of the reactors was also changed to the DeployInst, based on a recently merged PR in Cycamore (https://github.com/cyclus/cycamore/pull/529). These changes caused the changes to most of the files in this PR, and are very small changes. This PR is not actually as big as it may seem based on the number of files changed.
Finally, there is a small changed made to the
scripts/transition_metrics.py
file, because I began some of the analysis and was running into issues of duplicate entries by usingdf.pivot
. The remainder of the analysis will be included in a separate PR.