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

Fixing a couple of plots to use initial block height #1998

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

john-science
Copy link
Member

@john-science john-science commented Nov 1, 2024

What is the change?

Moving certain plots to use the BOL block height.

Why is the change being made?

This is to improve consistency in our block plots.

close #1158


Checklist

  • The release notes have been updated if necessary.
  • The documentation is still up-to-date in the doc folder.
  • The dependencies are still up-to-date in pyproject.toml.

@john-science john-science added the bug Something is wrong: Highest Priority label Nov 1, 2024
@john-science john-science requested a review from keckler November 1, 2024 17:00
armi/utils/plotting.py Outdated Show resolved Hide resolved
@john-science john-science requested a review from drewj-tp November 1, 2024 20:01
@keckler
Copy link
Member

keckler commented Nov 4, 2024

This looks fine, but I have not gotten a chance to test it with my real models.

Are you trying to get this merged within the coming week-ish? I don't have any strong need to get it merged for the coming release, but what are your priorities?

@john-science
Copy link
Member Author

@drewj-tp @keckler I know we're all busy, etc. So that's fine.

But I DID just get approval to merge this, if it gets Approved.

@keckler
Copy link
Member

keckler commented Dec 2, 2024

Trying to test now

Copy link
Member

@keckler keckler left a comment

Choose a reason for hiding this comment

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

Unfortunately, this does not appear to work right now. When I test it out on some dummy problem, I'm still getting the hot heights printed by plotAssemblyTypes() when it is called during the BOL reports interface (via newReportUtils.insertCoreAndAssemblyMaps()).

The issue appears to be that plotAssemblyTypes() is called based on blueprints, and block definitions in blueprints don't appear to have parents, and thus we get kicked into the except of the try/except clause that you added.

I think that, if we passed actual assemblies that have been constructed from the blueprints, then this will work? Let me know if that doesn't make sense.

armi/utils/plotting.py Show resolved Hide resolved
armi/utils/reportPlotting.py Show resolved Hide resolved
@terrapower terrapower deleted a comment from keckler Dec 3, 2024
@john-science john-science requested a review from keckler December 3, 2024 17:43
@keckler
Copy link
Member

keckler commented Dec 10, 2024

I think that this PR will break an existing usage where hot heights are desired.

def plotConvertedReactor(self):
"""Generate a radial layout image of the converted reactor core."""
bpAssems = list(self.convReactor.blueprints.assemblies.values())
assemsToPlot = []
for bpAssem in bpAssems:
coreAssems = self.convReactor.core.getAssemblies(bpAssem.p.flags)
if not coreAssems:
continue
assemsToPlot.append(coreAssems[0])
# Obtain the plot numbering based on the existing files so that existing plots
# are not overwritten.
start = 0
existingFiles = glob.glob(
f"{self.convReactor.core.name}AssemblyTypes" + "*" + ".png"
)
# This loops over the existing files for the assembly types outputs
# and makes a unique integer value so that plots are not overwritten. The
# regular expression here captures the first integer as AssemblyTypesX and
# then ensures that the numbering in the next enumeration below is 1 above that.
for f in existingFiles:
newStart = int(re.search(r"\d+", f).group())
if newStart > start:
start = newStart
for plotNum, assemBatch in enumerate(
iterables.chunk(assemsToPlot, 6), start=start + 1
):
assemPlotName = f"{self.convReactor.core.name}AssemblyTypes{plotNum}-rank{armi.MPI_RANK}.png"
plotting.plotAssemblyTypes(
self.convReactor.blueprints,
assemPlotName,
assemBatch,
maxAssems=6,
showBlockAxMesh=True,
)

That block of code calls to plotAssemblyTypes() to demonstrate the impact of the uniform mesh generator. In that circumstance, the hot heights are probably desired in the plots.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong: Highest Priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clashing prints on blueprints pictures
3 participants