Skip to content

Conversation

@gridley
Copy link

@gridley gridley commented Oct 28, 2023

The total weight per batch was accidentally being reset after each generation; the correct behavior is to sum between generations rather than assign from the previous generation's total weight. Now it is reset to zero only after each batch.

The bug is easy to show. Turn up the generations per batch on any eigenvalue problem.

@gridley gridley force-pushed the openmp-target-offload branch from a90105b to 4ca7d31 Compare October 28, 2023 23:34
@jtramm
Copy link

jtramm commented Nov 1, 2023

Nice -- thanks for finding this and putting in the fix! Just do double check, as we are now doing += instead of =, do we need to zero these values at the start of each batch (or was this already being done?)

@gridley
Copy link
Author

gridley commented Nov 1, 2023

Yes, total_weight gets zeroed out at the start of initialize_batch, which is precisely the desired behavior. The total weight should increase over the generations and reflect the total in the batch.

On the other hand, the other variables indeed are zeroed out in finalize_generation. So we maybe don't need to use an increment here. Might make more sense to remove the unnecessary zeroing in the other code and use assignment instead here; and this is something to also change up in the main OpenMC repo, too.

It would be so nice to have all these global parameters grouped into a struct to better keep track of them. It seems that total_weight is only used for eigenvalue estimation, and not in tallies themselves (they have their own total weight counter).

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.

2 participants