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

Some parameters after shuffing and rotating don't match reactor's state #1890

Open
drewj-tp opened this issue Sep 17, 2024 · 3 comments
Open

Comments

@drewj-tp
Copy link
Contributor

During rotation from shuffling and moving assemblies, some parameters will be wrong because they don't match the state of the reactor. And that's acceptable for now.

Originally posted by @drewj-tp in #1877 (comment)

If you grab an assembly and move it to a new location and don't edit any physics parameters, those parameters don't match the state of the reactor. Same for rotating an assembly. Things like linPowByPin and cornerFastFlux and even flux reflect the previous location and/or orientation of that assembly. It's possible to rotate an assembly and then grab physics parameters that look valid (because they have data) but don't reflect the current state of the reactor.

This is related to #1860 in that we need to rotate more parameters when we rotate assemblies and blocks. Both for shuffling and for growToFullCore when modeling a third core reactor and then expanding it to full core.

What to do

How can we signify that some parameters should not be used following a rotation or translation of an assembly? Separate from symmetric reflection and expansion. If we set them to arrays of zero or None, that's a way to invalidate the data.

@onufer
Copy link
Member

onufer commented Sep 17, 2024

the reason this is acceptable is that in most cases we run neutronics/TH after shuffling on the interface stack so wrong information is not written to the database

it would be safer to zero everything out though.

the main downside of zeroing everything out is that you could not use flux weighted average at BOC since there would be no flux.

@john-science
Copy link
Member

How can we signify that some parameters should not be used following a rotation or translation of an assembly? Separate from symmetric reflection and expansion. If we set them to arrays of zero or None, that's a way to invalidate the data.

ARMI provides locations and categories for things like this.

It feels like those are enough tools to solve the problem:

  • If a particular parameter doesn't have enough location/category, that is a failure of the parameter and we fix the parameter.
  • If we don't have sufficient .rotate() methods, we improve them (I'm thinking of your PR Drew).

I guess the question is... ARE location and category enough to solve the problem? Do you agree? If you do, we can close this ticket. Maybe open another ticket to fix any remaining, offending parameters we can find. If you don't, I want to hear it.

(Also, this ticket is dangerously close to a "Discussion" looking for "ideas", which is NOT a ticket. The ARMI Issues are my backlog of specific, identified tasks that I need to do. So I don't want to confuse my backlog of work with research and idea generation. Those are important! But for the "Discussion" area. Sorry, my backlog is just a large part of my day/life.)

@drewj-tp
Copy link
Contributor Author

ARMI provides locations and categories for things like this

We could maybe use or stretch the definition of Category.retainOnPlacement and/or Category.cumulative to determine what things to keep when rotating. But we also need to be careful of the difference between rotating an assembly because we've shuffled vs. rotating an assembly because we copied, moved, and rotated in during growToFullCore. @alexhjames introduced this in #1877 (comment)

ARE location and category enough to solve the problem? Do you agree? If you do, we can close this ticket.

Maybe but we still have gotten acceptance on what to do, if at all, for these parameters that don't match the reactor state post-rotation.

Also, this ticket is dangerously close to a "Discussion" looking for "ideas", which is NOT a ticket

I believe it's not a discussion because, as I understand the situation, ARMI is leaving the reactor in a not quite right / not quite wrong state post rotation. To me, that's a thing to catalogue. How we fix that, or if it's worth fixing at all, is a good topic for discussion with users and stakeholders.

If we don't have sufficient .rotate() methods, we improve them (I'm thinking of your PR Drew).

Yeah, it's very related to ongoing PR #1877. The accepted scope there is to rotate all pin-like parameters, even if it maybe doesn't make sense to keep them (ala this ticket)

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

No branches or pull requests

3 participants