search

njoy / NJOY2016

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

Arbitrarily large alpha and beta grids in LEAPR #317

Closed HunterBelanger closed 1 year ago

HunterBelanger commented 1 year ago

This PR allows LEAPR to be run with an arbitrarily large alpha and beta grid, without segfaulting. The lengths of the maxt and scr arrays are now determined based on nalpha and nbeta. It should close #292 .

There should also be an update the the manual for the error related to scr and mscr, as mscr is no longer hard coded to be 4000.

whaeck commented 1 year ago

Hi Hunter.

You should pull this into develop instead of master. We have been using develop as a staging area for new developments and fixes prior to release for a couple of years now. The master branch only gets updated when new versions get released (the HEAD of master now always points to the latest versioned release).

I have actually made changes to the size of maxt about a week ago (although I did not make the array allocatable) in #315. At the time, Skip did suggest making the array allocatable.

You should update your branch to be in line with the current develop branch (you will get conflicts since I made similar changes so you will need to resolve those too) and then change the branch to be merged into from master to develop. If you wish, I can make the changes myself since these are quite limited.

HunterBelanger commented 1 year ago

Hi Wim,

Sorry about that, I was apparently a potato when I made this PR. I have rebased my branch onto develop, and the PR should be pointing there as well now.

I went with making maxt allocatable, simply because from the source, it seemed clear that the max required size should be nbeta.

For writing out the ENDF file, mscr was also hard coded, and I didn't like the idea off that having a hard coded limit with maxt being allocatable, so I tried to figure out what the max size of scr needs to be. It seems to be working now for the grids and inputs I have been feeding it.

whaeck commented 1 year ago

Hi Hunter,

For some reason these changes seem to impact 4 tests in the non-regression suite. Since you are merging this from a fork, I cannot pull your changes and check the impact of the tests. I've put up a fix/leapr_array branch that we can merge your changes into (instead of develop) so that I can check the test results to see what is going on.

Can you change this PR to merge into fix/leapr_array instead of develop?

HunterBelanger commented 1 year ago

Hmm interesting. I have updated the branch to merge into.

My initial guess is that this is related to the size of maxt, given mscr should always be at least as large as it was previously. Maybe there is another location I missed in the module which requires maxt have a length larger than nbeta. I will try to look at that again to see if I can find if that is the case.

whaeck commented 1 year ago

That's what these test are for ;-)

I'm currently working on something else but once I'm done with that, I'll merge your branch in the fix/leapr_array so I can have a look myself.

whaeck commented 1 year ago

Hmm, seems like the tests did pass on Monday but not last week. I remember triggering the workflow on Monday but I must have assumed that the tests would continue to fail.