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
214 stars 82 forks source link

Fix verifyExpansion with derivedShape solvent #1647

Closed AidanMcDonald closed 2 weeks ago

AidanMcDonald commented 4 months ago

What is the change?

blockConverter._verifyExpansion() was throwing an error when the solvent was a derivedShape Component because isEncapsulatedBy requires the "other" component to have an "od" Parameter. I believe this check doesn't make sense for a derivedShape Component, so I added a simple check to skip the isEncapsulatedBy call in this case.

Why is the change being made?

This change is necessary for using the openmc plugin. Openmc cannot handle helix components so the openmc input writer uses MultipleComponentMerger, in which this bug arose.


Checklist

CLAassistant commented 4 months ago

CLA assistant check
All committers have signed the CLA.

john-science commented 3 months ago

Adding a unit test to prove the idea would be nice.

john-science commented 3 weeks ago

@AidanMcDonald Last I saw, you were able to continue with your work without this PR. Does that mean this PR isn't necessary?

AidanMcDonald commented 3 weeks ago

Correct, this PR is not necessary anymore. Sorry, I forgot to close this earlier. Please reject/abandon this PR. Thanks!

john-science commented 2 weeks ago

Closing this PR as unnecessary. Thanks, Aidan!