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

Adding spawn report to tally Admin #108

Merged

Conversation

valeriaRaffuzzi
Copy link
Member

A tally report for when fission neutrons are generated is added to the tallyAdmin and the tallyClerks.

This is called for every fission neutron generated in the different collision processors. In order to make this change, the interfaces of the collisionProcessor functions had to be changed to take tallyAdmin as an input.

This allowed to fix a problem in mgXsClerk: previously, chi was calculated in reportCycleEnd by looping over the fission source neutrons generated in a cycle. This worked well for eigenvalue problems but obviously it didn't work for fixed source ones. Now, in mgXsClerk, chi is calculated within reportSpawn, rather than reportCycleEnd, which works for every situation.

This PR also includes some spring cleaning! Some old files related to the tallies that were left in the folder but not used have been removed.

Copy link
Collaborator

@Mikolaj-A-Kowalski Mikolaj-A-Kowalski left a comment

Choose a reason for hiding this comment

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

Thank you for your work (I hope this one was not super dull since it required to change that much boilerplate code!)

Everything looks good to me, but personally I would be inclined to extend spawn report to cover all cases when a new particle is being created. This would sadly require adding some extra information in the report (e.g. MT number of the reaction which created the particle). I don't know what is your feeling on that?

@@ -57,6 +57,7 @@ module tallyClerk_inter
!! reportOutColl -> Process an outgoing from collision report
!! reportPath -> Process pathlength report
!! reportTrans -> Process transition report
!! reportSpawn -> Process fission report
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
!! reportSpawn -> Process fission report
!! reportSpawn -> Process report of a new particle being generated

I imagine we wish spawn to include extra creation mechanisms (e.g. N2N, splitting etc..)?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't really generate a new particle for N2N though, do we? I thought we just modify the weight, in which case I wouldn't call reportSpawn.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No, no we do not at all :-)

Sorry I just gave N2N as an example in case it was treated analogue at some point. I am in total agreement that there should be no spawn report when only the weight is changed. It should be only called when new, independent particle is created.

Comment on lines 324 to 337
!!
!! Process fission report
!!
!! See tallyAdmin_class for implicit assumptionas about the report.
!!
!! Args:
!! pOld [in] -> Particle that caused the fission event
!! pNew [in] -> Particle state of the fission neutron
!! xsData [inout] -> Nuclear Database with XSs data
!! mem [inout] -> Score Memory to put results on
!!
!! Errors:
!! Depend on specific Clerk
!!
Copy link
Collaborator

Choose a reason for hiding this comment

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

As above I don't think we wish to limit the spawn to include the fission only. We may need to use it for other instances of particle creation.

@@ -66,6 +67,7 @@ module tallyAdmin_class
!! reportOutColl -> Process post-collision reports in all clerks
!! reportPath -> Process pathlength reports in all clerks
!! reportTrans -> Process transition reports in all clerks
!! reportSpawn -> Process fission reports in all clerks
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
!! reportSpawn -> Process fission reports in all clerks
!! reportSpawn -> Process creation of new particle reports in all clerks

Comment on lines 591 to 603
!!
!! Process fission report
!!
!! Assumptions:
!! TODO: Decide on the details of this report
!!
!! Args:
!! pOld [in] -> Particle that caused the fission event
!! pNew [in] -> Particle state of the fission neutron
!!
!! Errors:
!! None
!!
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
!!
!! Process fission report
!!
!! Assumptions:
!! TODO: Decide on the details of this report
!!
!! Args:
!! pOld [in] -> Particle that caused the fission event
!! pNew [in] -> Particle state of the fission neutron
!!
!! Errors:
!! None
!!
!!
!! Process report that new particle is created
!!
!! Assumptions:
!! It should be send each time a new particle is created in the simulation by a nuclear reaction
!! or some other mechanism (e.g. splitting)
!!
!! Args:
!! pOld [in] -> Particle that caused the fission event
!! pNew [in] -> Particle state of the fission neutron
!!
!! Errors:
!! None
!!

!! Errors:
!! None
!!
recursive subroutine reportSpawn(self, pOld, pNew)
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 we should perhaps extend the interface of this report to take a MT number of a reaction that created the particle?
Basically I think it would be a good idea for a spawn report to cover all cases of particle creation, so we need to have some mechanism to filter the reaction we want.

For non-physical mechanisms (e.g. splitting) we could perhaps invent new fake MT numbers?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes fair, spawning from other reactions didn't even cross my mind! But we might need it in the future and it's a nice general tally to have.

I would say yes, we could call it also after splitting too.

Copy link
Collaborator

Choose a reason for hiding this comment

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

GitHub does not allow me to make comments next to unmodified code so it goes here:

Should we add a reportSpawn to the split procedure as well?

Copy link
Collaborator

@Mikolaj-A-Kowalski Mikolaj-A-Kowalski left a comment

Choose a reason for hiding this comment

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

Thanks! I think it is ok to merge.

@valeriaRaffuzzi valeriaRaffuzzi merged commit 58df62d into CambridgeNuclear:main Jan 25, 2024
5 checks passed
@valeriaRaffuzzi valeriaRaffuzzi deleted the fixedSourceMgXss branch January 25, 2024 15:39
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