arfc / predicting-the-past

The predicting the past project.
BSD 3-Clause "New" or "Revised" License
1 stars 3 forks source link

Cycamore reactor xml input generation and xml inclusion sample #16

Closed gyutaepark closed 7 years ago

gyutaepark commented 7 years ago

I have done the following so far and thought it would be a good point to make a PR to master before making the full simulation input file as described in #14 .

So far, the following have been achieved.

The explanations for xml inclusion have been added to the README for easy viewing.

katyhuff commented 7 years ago

Since all of the reactors are using the same in and out recipes, we don't need those recipes repeated in each of the reactor files. Can you make it so there is one file per reactor, containing only the contents (nothing about the recipe)?

The two recipes can be in just one file: uox_51.xml

gyutaepark commented 7 years ago

That change is now pushed. :)

gyutaepark commented 7 years ago

I did get includes to work with xincludes. I will check if Cyclus' native xml parser can parse xinclude but usually, the parser would have a separate function call for parsing xincludes.

Currently the xml include is disabled in the recipe_generator.py because it was originally used to merge s with . Since, all reactors have the same in and out_commod composition, the xml include was disabled to avoid redundancies.

I will check if cyclus can parse xml files with xincludes and go over the reviewed items. Thank you for checking on the work at such a late night. Did not expect for the code to be reviewed at this time. Sorry for such a late night pull request.

gyutaepark commented 7 years ago

I have currently finished most of the cyclus input file but am still missing the in and out_commod recipes. I will work on the recipes of all the input and output commods of all facilities that are currently missing in the cyclus input file tomorrow.

katyhuff commented 7 years ago

@gyutaepark : Please let me know when all of my review comments have been addressed and the PR is absolutely complete by re-requesting a review. Then, don't add new features to the PR until the review has been done. The scope of this PR is getting quite large, so any new features beyond the scope of this PR should occur in a separate pull request.

katyhuff commented 7 years ago

Specifically, regarding this comment:

"I have currently finished most of the cyclus input file but am still missing the in and out_commod recipes. I will work on the recipes of all the input and output commods of all facilities that are currently missing in the cyclus input file tomorrow."

The scope of this PR is getting quite large already, so any new features beyond the scope of this PR should occur in a separate pull request.

gyutaepark commented 7 years ago

I just realized that I made a big mistake on the DeployInst data. Instead of the build_time, I have accidentally placed reactor lifetime instead. I will fix this right now.

katyhuff commented 7 years ago

Nice catch! You're doing great! Thanks for working so quickly on all of this!!

On Fri, Jun 2, 2017, 3:21 PM gyutaepark notifications@github.com wrote:

I just realized that I made a big mistake on the DeployInst data. Instead of the build_time, I have accidentally placed reactor lifetime instead. I will fix this right now.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/arfc/predicting-the-past/pull/16#issuecomment-305886536, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYCqz3kCPWP9ZdMnLAyC_zxceICJf_Bks5sAGDSgaJpZM4NrFYV .