Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arbitrarily large alpha and beta grids in LEAPR #317

Merged
merged 7 commits into from
Oct 26, 2023

Conversation

HunterBelanger
Copy link
Contributor

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
Copy link
Member

whaeck commented Oct 18, 2023

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 HunterBelanger changed the base branch from master to develop October 19, 2023 12:49
@HunterBelanger
Copy link
Contributor Author

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
Copy link
Member

whaeck commented Oct 25, 2023

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 HunterBelanger changed the base branch from develop to fix/leapr_array October 26, 2023 13:43
@HunterBelanger
Copy link
Contributor Author

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
Copy link
Member

whaeck commented Oct 26, 2023

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
Copy link
Member

whaeck commented Oct 26, 2023

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.

Copy link
Member

@whaeck whaeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, merging this into a local branch for a bit more testing.

@whaeck whaeck merged commit 1668e55 into njoy:fix/leapr_array Oct 26, 2023
4 checks passed
@HunterBelanger HunterBelanger deleted the fix/leapr_array branch December 29, 2023 05:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Segmentation Fault for large alpha and beta grids
2 participants