njoy / NJOY2016

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

Fix/heatr moreio #284

Closed whaeck closed 1 year ago

whaeck commented 1 year ago

A small update to fix issue #283

The ENDF read routines do not read the entirety of TAB1 and LIST records by default. To do that, you need to do a "do while call moreio" after calling the tab1io and listio functions. In some modules this is not done rigorously - probably because it was assumed that the data was read in entirely anyway (e.g. Legendre coefficients were limited to just a few so no need to call moreio or tabulated mutliplicities are just a few values). While these assumptions have hold up, recent evaluations have started to break this. GROUPR was corrected in NJOY2016.69 for this very issue.

This time, it is HEATR's turn because newer evaluations use tabulated multiplicities that are quite large and the tab1io routine could not read these in a single pass.

In addition to this fix, I went down the rabbit hole and removed a few unused variables from acepn.f90.

NJOY2016 version number is set to 2016.70 and release notes were created for this future version (to be released around the end of March 2023).

whaeck commented 1 year ago

@kahlerac Can you have a look at this one?

kahlerac commented 1 year ago

Code changes look good, but for heatr at the start of subroutine hconvr there is a "allocate(scr(npage+50)) statement. With the new "moreio" calls later in this subroutine this allocation likely isn't large enough.

whaeck commented 1 year ago

Code changes look good, but for heatr at the start of subroutine hconvr there is a "allocate(scr(npage+50)) statement. With the new "moreio" calls later in this subroutine this allocation likely isn't large enough.

@kahlerac Good point. By what amount should we increase this? I did see a lot of npage+50 comparisons in heatr so we may want to replace that with a single variable so that its easier to increase later on.

kahlerac commented 1 year ago

Yes, this is one of those frustrating situations where today's big enough array will at some future date become too small. The most secure (and complex) answer would be to break the initial list or tab read into pieces ... with a "CONT" for the first six words and now "N2" tells us what we really need for the subsequent array. But that would require a lot of code changes/verification. As written now "npage+50" is only guaranteed to allow enough space for a single list or tab read and any subsequent "moreio" with an increasing array index risks an array overflow. The more expedient answer is to pick a best guess for "big enough". I'd think we could get by with a few thousand words, but maybe go for 10K or more and that should defer the issue until your successor is in charge! Skip

On Wed, Feb 22, 2023 at 10:49 AM Wim Haeck @.***> wrote:

Code changes look good, but for heatr at the start of subroutine hconvr there is a "allocate(scr(npage+50)) statement. With the new "moreio" calls later in this subroutine this allocation likely isn't large enough.

Good point. By what amount should we increase this? I did see a lot of npage+50 comparisons in heatr so we may want to replace that with a single variable so that its easier to increase later on.

— Reply to this email directly, view it on GitHub https://github.com/njoy/NJOY2016/pull/284#issuecomment-1440290029, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEHJISLDDEJK75L3WDLCGPDWYYYP7ANCNFSM6AAAAAAU6PLJ6Y . You are receiving this because you were mentioned.Message ID: @.***>

-- Dr. A. C. (Skip) Kahler Kahler Nuclear Data Services, LLC @.*** +1 321 368 3645

whaeck commented 1 year ago

@kahlerac I checked other modules in NJOY2016 and they all use npage+50 when allocating the scr array that read from the ENDF tape. We should talk about what to do here during our next meeting - which I'll try to attend (I promise).

whaeck commented 1 year ago

I'm merging this in develop since I need it in there. @kahlerac We'll talk about the npage+50 thing on a separate occasion.