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

NJOY2016.66 #226

Merged
merged 135 commits into from
Mar 10, 2022
Merged

NJOY2016.66 #226

merged 135 commits into from
Mar 10, 2022

Conversation

whaeck
Copy link
Member

@whaeck whaeck commented Nov 18, 2021

Release for NJOY2016.66

This has been under development for over a year - see release notes for more information.

whaeck and others added 30 commits April 6, 2020 15:10
…ve write subroutines from acefc.f90 to common space in anticipation of reuse
character(8),public::vers='2016.65'
character(8),public::vday='01Nov21'
character(8),public::vers='2016.66'
character(8),public::vday='18Nov21'
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 need to be updated due to my slow review?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, probably ...

Copy link
Member Author

Choose a reason for hiding this comment

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

What date do you want?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nevermind.

Copy link
Contributor

@nathangibson14 nathangibson14 left a comment

Choose a reason for hiding this comment

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

Fortunately, most of this code has been reviewed previously and has ample validation documentation, so my review was mostly a cursory code style review. I left a few comments, but they were minor at best.

I'll click approve now, but I'll also run a few more comparisons in ERRORR before you merge.

@@ -1080,8 +1161,15 @@ subroutine acephn(nendf,npend,nace,ndir,matd,tempd,iprint,mcnpx,&
xss(nex+5)=1
nex=nex+2+2*2
xss(last+2)=nex-dlwp+1
! amass=awr/awi
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you really want this commented code in the codebase?

Copy link
Contributor

Choose a reason for hiding this comment

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

hashtag valueadded

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 in at least 3 places.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm impressed you saw that :-)

I'm still leaving these comments because they represent the formulas for an arbitrary incident particle with a given mass awi. By rearranging and substituting awi = 0, you'll obtain the actual code implemented in these different places. This way, I'll always know where the formulas come from and how they were obtained - which has not been obvious in the past.

@whaeck whaeck merged commit b0ffb23 into master Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants