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

MG majorant precalculation for DT #8

Merged
merged 3 commits into from
Sep 2, 2022

Conversation

valeriaRaffuzzi
Copy link
Member

Precalculating the majorant is extremely simple for MG and it buys a nice 7-8% acceleration in, for example, a 2D C5G7 problem.

The majorant here is calculated in init and saved in mgBaseNeutronDatabase, instead of adding xss at every collision.

Copy link
Collaborator

@ChasingNeutrons ChasingNeutrons left a comment

Choose a reason for hiding this comment

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

Looks pretty good to me. I'm trying (and struggling) to think if there's really any circumstance that we wouldn't want this as you could in principle make it optional - probably not worth the extra code. Now we just need to do the same for CE (which I'm sure may be a bit more painful...).

Comment on lines 310 to 314
xs = ZERO
do i = 1,nMat
xs = max(xs, self % mats(i) % data(TOTAL_XS, g))
end do
self % majorant(g) = xs
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
xs = ZERO
do i = 1,nMat
xs = max(xs, self % mats(i) % data(TOTAL_XS, g))
end do
self % majorant(g) = xs
self % majorant(g) = maxval(self % mats(:) % data(TOTAL_XS, g)

But please check that it really does work. My Fortran is a bit rusty.

Copy link
Member Author

Choose a reason for hiding this comment

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

True Fortran magic

Comment on lines -129 to -133
xs = ZERO
do i=1,size(self % activeMats)
idx = self % activeMats(i)
xs = max(xs, self % getTotalMatXS(p, idx))
end do
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it may be a considerable problem that in the initialisation active materials are not considered.
Unless I am missing something (quite possible -> Friday evening ;-) ) the pre-calculated majorant is based on all defined materials, not only the materials which are actually included in the geometry.

I would suggest moving the majorant calculation to the activate subroutine.

Copy link
Member Author

Choose a reason for hiding this comment

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

Very good point. I will do that!

@Mikolaj-A-Kowalski
Copy link
Collaborator

Thanks for the development! It is really nice optimisation!

I absolutely love the documentation and corrections to the typos 🦭

@valeriaRaffuzzi
Copy link
Member Author

Done!
@ChasingNeutrons I don't think there's any obvious disadvantage, it doesn't take much memory or time to store it even if ST is being used. And it's so simple to do that it was a shame not having it by default!

@valeriaRaffuzzi valeriaRaffuzzi merged commit 771e2a0 into CambridgeNuclear:main Sep 2, 2022
@valeriaRaffuzzi valeriaRaffuzzi deleted the MgMajorantDT branch September 2, 2022 20:37
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.

3 participants