njoy / NJOY2016

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

Fix potential divide-by-zero when ihave=2 in PURR #51

Closed paulromano closed 6 years ago

paulromano commented 6 years ago

This pull request avoids a potential divide-by-zero exception when self-shielding heating values in PURR (when a user has supplied partial kermas for MT=2,18,102 from HEATR -- ihave=2). Notice that on one branch of the if (lssf.eq.1) there was already a check for a zero value in the denominator; it just didn't appear in the other branch and I was unlucky enough to hit it. I ran into this trying to process 210Po from ENDF/B-VIII.beta6 using the following input:

reconr /
20 21
'ENDF/B-8.0 PENDF for  84-Po-210 '/
8437 2/
0.001/ err
'ENDF/B-8.0:  84-Po-210 '/
'Processed by NJOY'/
0/
broadr /
20 21 22
8437 1 0 0 0. /
0.001/
293.6
0/
heatr /
20 22 23 /
8437 3 /
302 318 402 /
purr /
20 23 24 /
8437 1 1 20 64 /
293.6
1.e10
0/
acer /
20 24 0 25 26
1 0 1 .01 /
'ENDF/B-8.0:  84-Po-210  at 293.6'/
8437 293.6
1 1/
/
stop
jlconlin commented 6 years ago

Sorry it has taken so long to respond here.

We have a new, informal, way of giving version numbers to NJOY2016. When this is ready to be merged (as this one now is), the version number and date need to be updated in the version.f90 file. Please update that (according to what we have put in the master branch and we'll take a look and (probably) approve it.

paulromano commented 6 years ago

Thanks @jlconlin. Just incremented the version number and date.

whaeck commented 6 years ago

Hi Paul, I have just started looking at this. I am running your input to see it break down. The fix looks clean, it will solve the issue but I still would like to know why a(n1+j+nbin) is equal to zero in the first place? In addition, this might warrant a warning/error. I will start looking into the source code to find out more. Also, maybe we should compare to a very small number, and not zero itself? Best regards, Wim

whaeck commented 6 years ago

Following our phone call, this change was added to another branch that fixes some additional issues in purr. As a result, we close this pull request without merging. Thanks again for pointing this out to us.

paulromano commented 6 years ago

Thanks for digging, @whaeck!