njoy / NJOY2016

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

Updating constants to the CODATA2018 values. #140

Closed jlconlin closed 3 years ago

jlconlin commented 5 years ago

Not all tests are passing. Need to have a conversation about how best to proceed here.

Also, don't merge until CSEWG approves my proposal to the ENDF manual.

jlconlin commented 3 years ago

So, this should be ready to go. Please double check the physical constants in the phys.f90 file.

The reference tapes were created by just running the tests and copying the output tapes over. Note that's how the original reference tapes were created anyway.

whaeck commented 3 years ago

Yeah, but it fails in actions. Probably because of the python version - I'm updating that.


From: Jeremy Lloyd Conlin notifications@github.com Sent: Wednesday, January 6, 2021 10:06:24 AM To: njoy/NJOY2016 Cc: Haeck, Wim; Review requested Subject: [EXTERNAL] Re: [njoy/NJOY2016] Updating constants to the CODATA2018 values. (#140)

@jlconlin commented on this pull request.


In tests/execute.pyhttps://github.com/njoy/NJOY2016/pull/140#discussion_r552814809:

@@ -45,8 +46,9 @@ def lineEquivalence(ref, trial, n, relativeError=1E-9, absoluteError=1E-10): else:

Look at all the floats on the lines

for rF, tF in zip(refFloats, trialFloats):

  • equal = fuzzyDiff(makeFloat(rF), makeFloat(tF),
  • relativeError, absoluteError)
  • equal = isclose( makeFloat(rF), makeFloat(tF),

I much prefer using a built-in function than home-grown.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHubhttps://github.com/njoy/NJOY2016/pull/140#pullrequestreview-562884721, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AENOHQB665ALGPUWTYXNOMDSYSKBBANCNFSM4JINVWRQ.

whaeck commented 3 years ago

Non-regression tests should be verified prior to updating the test results to assess the impact of changing physical constants - which are expected to be small - but it needs to be done for proper QA.

To perform these tests, I updated the execute.py for these tests, now using Python's own isclose function to compare two numbers. By default, the relative difference must be greater than 1e-9 for two numbers to be flagged as different, as long as one of those is not zero. In that case, the absolute difference may be up to 1e-10 for these numbers to be considered equal. At those settings, 47 out of 58 tests are flagged as failed.

At a relative difference of 1e-9, ENDF formatted numbers (with 6 digits after the decimal point) that differ even in their last digit get flagged as different. Setting the relative difference to 1e-5 and 5e-5 will allow us to filter out the cases where the difference is in the last significant digit in standard ENDF notation. At 1e-5 relative difference, only 14 out of 58 tests still continue to be flagged as "failed".

These tests are: 1, 2, 11, 12, 14, 16, 19, 21, 22, 25, 32, 37, 39 and 49. In what follows, I will detail these remaining differences to show why these do not constitute major blocking issues.

whaeck commented 3 years ago

Test 12 and 14: only differences in VIEWR produced post-script files - acceptable

The comparison of all reference tapes, except for the post script files, comes out correctly. In the postscript files themselves only a few numbers get flagged as different. For example, for test 12:

*** referenceTape24 ***
--- tape24 ---
***************
*** 1464 ***
!   341.10   169.01 lineto
--- 1464 ---
!   341.09   169.01 lineto
whaeck commented 3 years ago

Test 1: fixed versus scientific notation - acceptable

A single reference tape has ~500 different lines due to a difference in fixed and scientific notation (the numbers are actually the same). For example:

*** referenceTape25 ***
--- tape25 ---
***************
*** 3414 ***
!-8.970661-1-6.850674-1-4.629149-1-2.296181-1 1.621671-2 2.761011-11306 6221  100
--- 3414 ---
!-8.970661-1-6.850674-1-4.629149-1-2.296181-1 1.621672-2 0.276101121306 6221  100
whaeck commented 3 years ago

Test 2, 11 and 16: small differences in scattering matrices - acceptable

The differences are detected in GENDF files, and more specifically in the matrices (stored under MF6). These constitute differences in the 4th or 5th digit, and only if the number are already quite low (below 1). It only happens for 5 to 15 values for each of the quoted tests. For example:

*** referenceTape29 ***
--- tape29 ---
***************
*** 1556 ***
!-4.672910-4-6.046823-6 3.536490+1 5.279371-1 1.773018-3 1.910515-51050 6  2  218
--- 1556 ---
!-4.672980-4-6.046867-6 3.536490+1 5.279371-1 1.773016-3 1.910515-51050 6  2  218
whaeck commented 3 years ago

Test 19, 37 and 39: probability tables are slightly different - acceptable

The differences are detected in NJOY PENDF files for MF2 MT153, the internal storage section for probability tables.Changing the constants has an influence on cross section values, so some changes are to be expected. Only three of the ptable tests in the test suite (there's at least 10 tests related to ptables as far as I know) exhibit these differences, which indicates that this is not a consistent problem.

For example:

*** referenceTape29 ***
--- tape29 ---
***************
*** 18578 ***
! 4.640000-2 7.860000-2 8.727500-2 8.575000-2 7.685000-2 9.235000-29443 2153  750
--- 18578 ---
! 4.637500-2 7.862500-2 8.727500-2 8.575000-2 7.685000-2 9.235000-29443 2153  750
whaeck commented 3 years ago

Test 21 and 32: output files appear to have different number of lines - might require further investigation

In this case, the test fails because the reference file and new file do not have the same number of lines. Most likely some tolerance may have lead to the addition of an additional point or something.

whaeck commented 3 years ago

Test 22, 25 and 49: thermal scattering related

Test 22 is a LEAPR test. The diferences are detected in the ENDF file produced by LEAPR, in MF7 MT4 for about 25 lines in a file of over 4500 lines. These are mainly in the 4th and 5th digit after the decimal point. This is acceptable.

Test 25 is a test for producing ACE files for H in H2O for 3 temperatures. The differences occur only in the ACE file for 300 K. There's about 1000 lines with differences for a file containing of the order of 12500 lines. These are mainly in the 4th and 5th digit after the decimal point.

Test 49 is a test for Zr in Zrh. The resulting ACE files appear to have a different number of lines.

jlconlin commented 3 years ago

Test 1: fixed versus scientific notation - acceptable

A single reference tape has ~500 different lines due to a difference in fixed and scientific notation (the numbers are actually the same). For example:

*** referenceTape25 ***
--- tape25 ---
***************
*** 3414 ***
!-8.970661-1-6.850674-1-4.629149-1-2.296181-1 1.621671-2 2.761011-11306 6221  100
--- 3414 ---
!-8.970661-1-6.850674-1-4.629149-1-2.296181-1 1.621672-2 0.276101121306 6221  100

I was hoping that the relative diff would catch these kinds of differences. Perhaps not?

whaeck commented 3 years ago

Since the comparison tool does not distinguish between ENDF and ACE, etc., the MAT number gets taken as part of the value in the comaprison. The comparison that gets flagged here are these two pieces:

2.761011-11306
0.276101121306

0.276 is indeed different from 2.76e-11306

jlconlin commented 3 years ago

Since the comparison tool does not distinguish between ENDF and ACE, etc., the MAT number gets taken as part of the value in the comaprison. The comparison that gets flagged here are these two pieces:

2.761011-11306
0.276101121306

0.276 is indeed different from 2.76e-11306

Oh yes, of course. I forgot about that mess.

jlconlin commented 3 years ago

Thanks @whaeck for your detailed review on this.