njoy / NJOY2016

Nuclear data processing with legacy NJOY
https://www.njoy21.io/NJOY2016
Other
96 stars 86 forks source link

modified aceth.f90 by D. Roubtsov #44

Closed danrouzen closed 6 years ago

danrouzen commented 6 years ago

modifications: for iwt=2 option (thermal ace file generation), there is a bug in subroutine acesix ; see "do loop", lines 417 - 430.
Our point is that, if iwt=2, it is NOT correct to use "xbar" to control the exit from this loop. Without this fix, one can have (unexpected) warning messages ( ---warning from acesix---) that the scattering cosines are outside (-1.0 , 1.0). These warnings (check by keyword "warning") were slightly modified and added to subroutine acesix ; they were commented out in the official versions of NJOY2012 and NJOY 2016. dan.roubtsov@cnl.ca (office) danrouzen@gmail.com (home)

whaeck commented 6 years ago

Good morning Dan,

Thank you for sharing your fix. Before I can approve the pull request, there are a couple of things I would like to ask you.

Did you by any chance use a Windows text editor to make these modifications in your fork? As it stands now, every line in the file is flagged as changed (this is probably due to end of line characters that were added by the text editor). In order to keep the proper history of the code, we will need to correct this by removing the offending characters and committing the changes to your fork. I have taken the liberty of stripping the characters for you. Below you will find a link to a zip file containing the stripped file which you can commit on your end (I do not think I can do that on your fork).

aceth.f90.zip

I would also like to ask you if you have an example case for which this occurs so that I can test the changes on our side as well. I would only need an njoy input file, as long as the thermal scattering ENDF file used in the example can be found on the NNDC website or elsewhere.

Best regards, Wim

danrouzen commented 6 years ago

Hi Wim:

I will follow your advice, thank you.

meanwhile, to test it (and be convinced that there is a problem in aceth), I suggest the following way: we use NJOY2016 with current aceth (version1) [ aceth.f90 ] NJOY2016 with aceth with warnings only (version2) [I named it "aceth_warning.f90" ] NJOY2016 with aceth with warnings and the fix (version3) [I named it "aceth_warning_fix.f90" ]

I suggest two examples, from recent B-VIII.0beta5 testing, input_nj16_h1_h2o_acer_293p6k_32_c input_nj16_d_d2o_ace_293p6k_32_c (and they are not a kind of extreme cases with nbin ~ 60 in thermr).

with NJ2016_version2, you will see suspicious warnings from acer (aceth_warning.f90) about the cosines it fixed; these warnings will disappear with NJ2016_verion3.

a story behind this bug is in "Roubtsov_et_al_B80b5_CSEWG2017_extended.pdf"; see slides 20-24.

regards, Dan

.

On Mon, Nov 27, 2017 at 2:16 PM, Wim Haeck notifications@github.com wrote:

Good morning Dan,

Thank you for sharing your fix. Before I can approve the pull request, there are a couple of things I would like to ask you.

Did you by any chance use a Windows text editor to make these modifications in your fork? As it stands now, every line in the file is flagged as changed (this is probably due to end of line characters that were added by the text editor). In order to keep the proper history of the code, we will need to correct this by removing the offending characters and committing the changes to your fork. I have taken the liberty of stripping the characters for you. Below you will find a link to a zip file containing the stripped file which you can commit on your end (I do not think I can do that on your fork).

aceth.f90.zip https://github.com/njoy/NJOY2016/files/1507218/aceth.f90.zip

I would also like to ask you if you have an example case for which this occurs so that I can test the changes on our side as well. I would only need an njoy input file, as long as the thermal scattering ENDF file used in the example can be found on the NNDC website or elsewhere.

Best regards, Wim

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/njoy/NJOY2016/pull/44#issuecomment-347294057, or mute the thread https://github.com/notifications/unsubscribe-auth/Abjiuuf2_zduS2WEe7sa7LSGCiggMXOIks5s6wqHgaJpZM4Qonie .

whaeck commented 6 years ago

The pull request for the feature/acer-endf8 branch has been made and includes these changes. This pull request can thus be closed.

Thanks again for the help.

Best regards, Wim