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
232 stars 90 forks source link

Its unclear how to set symmetry correctly #589

Closed onufer closed 2 years ago

onufer commented 2 years ago

I converted a 1/3 core reactor to full core using ThirdCoreHexToFullCoreChanger.convert

After expanding the symmtry is set using

r.core.symmetry = geometry.SymmetryType(
    geometry.DomainType.FULL_CORE, geometry.BoundaryType.NO_SYMMETRY
)

My code also tests weather its full core using

if self.r.core.isFullCore

which checks

return self.symmetry.domain == geometry.DomainType.FULL_CORE

Since core.symmetry was set, but self.symmetry.domain is checked for symmetry, it still believes it is not full core

Please advise why both core.symmetry and self.symmetry.domain exist, and how to communicate to ARMI that it is now full core.

john-science commented 2 years ago

For your first question:

why both core.symmetry and self.symmetry.domain exist...

If you do a quick type() on those variables, you will find that r.core.symmetry is of type SymmetryType:

https://github.com/terrapower/armi/blob/035711afd1514ec5914c59c3cf898dfb3213c5a4/armi/reactor/geometry.py#L280-L282

And r.core.symmetry.domain is of type DomainType:

https://github.com/terrapower/armi/blob/035711afd1514ec5914c59c3cf898dfb3213c5a4/armi/reactor/geometry.py#L125-L127

So, to answer your question "why both core.symmetry and self.symmetry.domain exist", that is because the SymmetryType has one more important piece of information than just DomainType; it contains BoundaryType information. That answers the question: is the boundary periodic or reflective? That's all.

john-science commented 2 years ago

For your second question:

how to communicate to ARMI that it is now full core.

Perhaps this unit test will be a good example of how that geometry changer is supposed to work:

https://github.com/terrapower/armi/blob/035711afd1514ec5914c59c3cf898dfb3213c5a4/armi/reactor/converters/tests/test_geometryConverters.py#L317-L333

I ran this unit test locally, and on line 332, I added in this line:

self.assertTrue(self.r.core.isFullCore) 

and the test still passed. It looks to me like this ThirdCoreHexToFullCoreChanger.convert() works as expected. There isn't a bug (at least not an obvious one). So, are you seeing something different here? Did you perhaps uncover some bug using very specific inputs?

onufer commented 2 years ago

at the end of your unit test, can you test self.r.core.isFullCore is true. that would be where a possible bug would live

john-science commented 2 years ago

@onufer I have ammended the unit test as follows, and it still passes:

    def test_growToFullCoreFromThirdCore(self):
        """Test that a hex core can be converted from a third core to a full core geometry.""" 
        # Check the initialization of the third core model  
        self.assertFalse(self.r.core.isFullCore)  
        self.assertEqual( 
            self.r.core.symmetry,   
            geometry.SymmetryType( 
                geometry.DomainType.THIRD_CORE, geometry.BoundaryType.PERIODIC 
            ), 
        ) 
        initialNumBlocks = len(self.r.core.getBlocks()) 
        # Perform reactor conversion 
        changer = geometryConverters.ThirdCoreHexToFullCoreChanger(self.o.cs) 
        changer.convert(self.r) 
        # Check the full core conversion is successful 
        self.assertTrue(self.r.core.isFullCore)    # NOTE: This is new.
        self.assertGreater(len(self.r.core.getBlocks()), initialNumBlocks)  
        self.assertEqual(self.r.core.symmetry.domain, geometry.DomainType.FULL_CORE) 
        # Check that the geometry can be restored to a third core  
        changer.restorePreviousGeometry(self.o.cs, self.r) 
        self.assertEqual(initialNumBlocks, len(self.r.core.getBlocks())) 
        self.assertEqual(  
            self.r.core.symmetry,  
            geometry.SymmetryType( 
                geometry.DomainType.THIRD_CORE, geometry.BoundaryType.PERIODIC 
            ),  
        ) 
        self.asserFalse(self.r.core.isFullCore)  # NOTE: This is new.

So the basic unit test checks out. Is this not what you expected? OR do you see something different with your data?

onufer commented 2 years ago

Thanks for explanation, issues is probably on my end