njoy / NJOY2016

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

Fix/acer consis #147

Closed whaeck closed 4 years ago

whaeck commented 4 years ago

This addresses issue #130

This relates to changes made by the consistency checks when a checker acer run is requested. NJOY only rarely modifies data. It does so for secondary particle distributions that use LAW=4 (isotropic angular distribution and continuous tabulated energy distributions) or LAW=44 (Kalbach-Mann).

When the consis routine determines the maximum outgoing energy is too high compared to epmax, it sets the last value of the outgoing energy array to this value, and all values before it to monotonically increasing values that are lower than epmax. It then adjusts the pdf values to maintain the associated cdf. This often leads to the introduction of artificial spikes in the plots produced by acer.

This behaviour has now been modified. Whenever this problem is detected, the consis routine will now reset the first energy larger to epmax to a value slightly below it and adjust the pdf value so that the cdf value for this energy point is 1.0. All other energy points after this energy point along with their associated data is set to zero. The locators in the xss array are left unchanged.

Due to this new behaviour, and because the routine that writes the xss array does not check locators, the change routine was modified as well to verify consistency of the locators with the actual position in the xss array. Whenever the locator is inconsistent, it will either just advance to the locator (thus taking into account the gaps created by the consis routine) or issue an error when the locator points to a position lower than the current one.

The change routine was also simplified to improve its readability by adding subroutines to write a list of integers, a list of real numbers, a single integer and a single real number.

A large number of compiler warnings were also fixed in this pull request (mostly unused or uninitialised variables).

Test case 55 was added to properly test this for a LAW=44. Up to now, no case for LAW=4 has been found.

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at ?% when pulling 7fde55b6dfd309b6632a470d2f8a6db591463a95 on fix/acer-consis into d0cab9119a68104fb0ae8f4f57248181565ce218 on master.

whaeck commented 4 years ago

@jlconlin If you don't have enough pull requests, here's another :-)

whaeck commented 4 years ago

@nathangibson14 The reason why we go from the old treatment in consis to the new treatment was to clean up some of the graphs we were producing. If you had 2 eprime points that were above the calculated epmax (let's say this is 1e-10 MeV), njoy set the last two eprimes to 9.999999999998e-11 and 9.999999999999e-11 (give or take a few 9s). The delta E for these bins is so small that the probability explodes. As a result, the graph only shows spikes. See #130 for an example.

nathangibson14 commented 4 years ago

Aha, I see what I was missing. The sigfig routine is changing the value from epmax. And to be clear, this shouldn't meaningfully change the values produced in downstream calculations sampling, it just makes prettier plots. I'm happy with that.

whaeck commented 4 years ago

One could say it is a cosmetic change with a little more changes behind it ;-)

It did allow me to write up the remainder of the continuous energy ACE specification, maybe I should do that pull request next.

jlconlin commented 4 years ago

Do we know if @jchsublet has had a chance to try this and whether or not it fixes issue #130

jchsublet commented 4 years ago

@whaeck @jlconlin yes you should know that jchsublet tested parts of this branch as he unadvertantly use it as is njoy2016.53 code when released see my last comment on #130. Having said that I'll be happy to test any other release when it occurs or even a test one

p.s. your timeline is too long between bug report and correction to be efficient

jlconlin commented 4 years ago

Thanks @jchsublet We need to get better at speeding up.