terrapower / armi

An open-source nuclear reactor analysis automation framework that helps design teams increase efficiency and quality
https://terrapower.github.io/armi/
Apache License 2.0
219 stars 87 forks source link

Add testing for uniform pitch change to the reactor core. #848

Open jakehader opened 2 years ago

jakehader commented 2 years ago

As for testing the pitch or tying pitch changes to a specific component this is outside of the scope of this change. I grepped around the code and don't see any tests for core::setPitchUniform or block.setPitch that tests the conservation of solid component mass. I would like to see a separate PR that adds testing to these two methods (on the reactor core is fine) with and without the axial expansion changer.

https://github.com/terrapower/armi/blob/86dc4d8262cf3a30811f22c27eeb9556b004d611/armi/reactor/reactors.py#L2143

For example:

I want to just check that (1) the pitch change is being propagated correctly at that there are tests for this and (2) that the pitch change with thermal expansion doesn't run into issues if it is called after the axial expansion changer and that the results are correct in terms of changes in masses and volume fractions of each component (see: b.getVolumeFractions())

Originally posted by @jakehader in https://github.com/terrapower/armi/issues/845#issuecomment-1223316296

albeanth commented 2 years ago

😄 I was literally just drafting out an issue for this! You beat me to it!

john-science commented 5 months ago

@jakehader This ticket is still assigned to you. That's cool with me, but if you aren't planning on working on it please un-assign it.

(You are not being singled out, I am going through all the ARMI tickets.)

albeanth commented 5 months ago

@john-science we should probably have a requirement and associated testing for updating and setting the core plate grid pitch. This would be very useful for both hex and cartesian grid based reactors and properly documenting this functionality (it's abilities/limitations/etc) would be nice.

@keckler @mgjarrett for visibility and input in case I've forgotten something we have internally providing coverage for this functionality.

keckler commented 5 months ago

I do not think we have any tests that are specific as what Jake is asking for. We have a requirement that one shall be able to do expansion/contraction for the purposes of reactivity coefficient calculation, but not that specific masses/volumes are preserved.