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

Fix bugs in process_j312.sh #100

Merged
merged 21 commits into from
Jan 28, 2022
Merged

Fix bugs in process_j312.sh #100

merged 21 commits into from
Jan 28, 2022

Conversation

yardasol
Copy link
Contributor

@yardasol yardasol commented Jan 18, 2022

Summary of changes

This PR fixes the bugs described in #99
This PR also renames process_j312.sh to process_j312.bash to clarify that the script uses bash syntax, and will not work properly (or even fail) when run as a native shell script.
This PR also adds detection of existing compressed cross section library files, neutron data files, and decay chain file to reduce time it takes to rerun the script by skipping the download commands.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Required for Merging

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
    • CI tests pass
    • Local tests pass (including Serpent2 integration tests)

Associated Issues and PRs

Associated Developers

Checklist for Reviewers

Reviewers should use this link to get to the
Review Checklist before they begin their review.

@yardasol yardasol requested review from abachma2, munkm and lkissin2 and removed request for lkissin2 and abachma2 January 18, 2022 22:36
@yardasol
Copy link
Contributor Author

block merging until #103 is resolved.

@abachma2 abachma2 self-requested a review January 25, 2022 18:46
Copy link
Contributor

@abachma2 abachma2 left a comment

Choose a reason for hiding this comment

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

Overall this looks good. I did try this out and it does work well. I just have the one question about JEFF versions.

Comment on lines +35 to +39
# Get neutron induced and spontaneous fission yield data from JEFF 3.3
if [[ ! -f $DATADIR/sss_jeff33.nfy ]]
then
wget -O $DATADIR/sss_jeff33.nfy https://www.oecd-nea.org/dbdata/jeff/jeff33/downloads/JEFF33-nfy.asc
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this pull data from JEFF 3.3? I thought this was for getting data from JEFF 3.1.2 ???

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is no spontaneous or delayed neutron fission data in JEFF 3.1.2, nor is there decay data. The data from the JEFF 3.3 library is backwards compatible with the JEFF 3.1.2 library.

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense. Just wanted to clarify the reason for the version change.

Copy link
Contributor

@abachma2 abachma2 left a comment

Choose a reason for hiding this comment

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

I approve this pull request. I will wait to merge until #103 is resolved.

@yardasol yardasol merged commit 88c5c94 into arfc:master Jan 28, 2022
github-actions bot pushed a commit to yardasol/saltproc that referenced this pull request Jan 29, 2022
@yardasol yardasol added this to the 0.4.0 milestone Jan 29, 2022
@yardasol yardasol deleted the fix-j312 branch March 9, 2022 19:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: bugs in process_j312.sh
2 participants