r.core.growToFullCore() error when loading from database

[jasonbmeng]

Discussed with @jakehader. When loading a reactor using a case settings file, mechanical team is able to grow to full core. When loading a reactor object from a database, a runtime error is thrown when growing to full core. It appears that with the latter method, the global assembly number is set incorrectly.

Works:

import armi
o = armi.init(fName='myCaseSettings.yaml')
o.r.core.growToFullCore(cs)

Throws RuntimeError:

from armi.bookkeeping.db import databaseFactory
db = databaseFactory('myDatabaseName.h5','r')
with db:
    r = db.load(5,2)
r.core.growToFullCore(cs)

[jakehader]

To fix this the database3 update global assembly number function should be called on the database load function. This is automatically done on the database interface loadState but isn't called if you just want to load a reactor with .load

[john-science]

@jasonbmeng It would really help me fix this bug if you could copy/paste the actual error / stack trace below. Thanks!

@jakehader Are you saying this is a known bug, because this line is called when loading a reactor from DB, not when normally initializing a reactor?

armi/armi/bookkeeping/db/database3.py

Lines 144 to 147 in a3f23ed

	def updateGlobalAssemblyNum(r):
	assemNum = r.core.p.maxAssemNum
	if assemNum is not None:
	assemblies.setAssemNumCounter(int(assemNum + 1))

[jasonbmeng]

Sure thing, @john-science. Here's the stack trace when running r.core.growToFullCore(cs):

In [9]: r.core.growToFullCore(cs)
[xtra] No edge assemblies to remove
[info] Expanding to full core geometry
[err ] The assembly <radial shield outer Assembly A0155 at 013-016> in the reactor already has the name A0155.
       Cannot add <radial shield outer Assembly A0155 at 013-019>. Current assemNum is 156
---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
<ipython-input-9-0b3aa91bbdb3> in <module>
----> 1 r.core.growToFullCore(cs)

~\codes\nala\framework\armi\reactor\reactors.py in growToFullCore(self, cs)
   2138
   2139         converter = gc.ThirdCoreHexToFullCoreChanger(cs)
-> 2140         converter.convert(self.r)
   2141
   2142         return converter

~\codes\nala\framework\armi\reactor\converters\geometryConverters.py in convert(self, r)
   1345                 newAssem = copy.deepcopy(a)
   1346                 newAssem.makeUnique()
-> 1347                 r.core.add(newAssem, r.core.spatialGrid[i, j, 0])
   1348                 self._newAssembliesAdded.append(newAssem)
   1349

~\codes\nala\framework\armi\reactor\reactors.py in add(self, a, spatialLocator)
    536                 )
    537             )
--> 538             raise RuntimeError("Core already contains an assembly with the same name.")
    539         self.assembliesByName[aName] = a
    540         for b in a:

RuntimeError: Core already contains an assembly with the same name.

[jakehader]

@jasonbmeng It would really help me fix this bug if you could copy/paste the actual error / stack trace below. Thanks!

@jakehader Are you saying this is a known bug, because this line is called when loading a reactor from DB, not when normally initializing a reactor?

armi/armi/bookkeeping/db/database3.py

Lines 144 to 147 in a3f23ed

	def updateGlobalAssemblyNum(r):
	assemNum = r.core.p.maxAssemNum
	if assemNum is not None:
	assemblies.setAssemNumCounter(int(assemNum + 1))

I am saying that I worked with Jason when he was having this issue and the load function on the database3 appears to not call that update assembly numbers (if I remember correctly now).

[john-science]

@jakehader @jasonbmeng

I went in to fix this bug this morning, and was unable to reproduce it.

For example, please see these unit tests where I try to reproduce the bug:

john-science@dff0977

[jakehader]

Dang okay. @jasonbmeng you might need pass the problem or failing case to @john-science internally. This might be a combination of things leading to the problem then.

[john-science]

Thanks to Jeff, I was able to reproduce this bug with a database he gave me.

I will take on solving the bug. Though I still can not generate a pure-ARMI unit test that reproduces this bug. And that would obviously be helpful for both debugging and as a final test in the solution PR.

[jeffbaylor]

John,

This test demonstrates the issue. It passes as written, throwing a RuntimeError the first two attempts and then succeeding in growing to a full core on the third attempt. This should be a pure-ARMI solution.

class TestDbFactoryGrowToFullCore(unittest.TestCase):
    def _loadAndGrow(self):
        with databaseFactory('myDatabase.h5', "r") as dbo:
            r = dbo.load(0, 0, allowMissing=True)
        r.core.growToFullCore(None)

    def testDbFactoryGrowToFullCoreError(self):
        with self.assertRaises(RuntimeError):
            self._loadAndGrow()
        with self.assertRaises(RuntimeError):
            self._loadAndGrow()
        self._loadAndGrow()

[john-science]

This test demonstrates the issue. It passes as written, throwing a RuntimeError the first two attempts and then succeeding in growing to a full core on the third attempt. This should be a pure-ARMI solution.

Yeah, I have various versions of this, but they pass unless provided special databases:

john-science@dff0977

Interestingly, the default ARMI test data doesn't seem to reproduce this bug. This is a 1/3 core situation:

armi/armi/bookkeeping/tests/test_databaseInterface.py

Line 206 in 4fcea82

class TestDatabaseReading(unittest.TestCase):

For example, I can generate the default H5 file used in dozens of unit tests, but the test passes without generating the error:

def test_growToFullCoreFromFactory(self):
    from armi.bookkeeping.db import databaseFactory

    db = databaseFactory(self.dbName, "r")
    with db:
        r = db.load(0, 0, allowMissing=True)

    r.core.growToFullCore(None)

So, that's fun.

[jeffbaylor]

Have you run that test method by itself? The reason all of the mech unit tests passed was because we were loading databases many times before the process got to the one that tested the full core growth. When I run the grow to full core unit test method only, it fails. I didn't try all of our test databases, but the two I tried both exhibited this problem.

[john-science]

Have you run that test method by itself? The reason all of the mech unit tests passed was because we were loading databases many times before the process got to the one that tested the full core growth. When I run the grow to full core unit test method only, it fails. I didn't try all of our test databases, but the two I tried both exhibited this problem.

Yeah, as my personal workflow goes, I am only running this one test. You can easily run one test by doing:

pytest armi/bookkeeping/tests/test_databaseInterface.py::TestDatabaseReading::test_growToFullCoreFromFactory

Just to be clear:

1. You totally proved to me this is a real bug.
2. I can debug with the file you gave me.

I was just hoping to simplify the process in (2) with an ARMI-only test case. And I"m a little worried that I won't be able to write a decent unit test for my PR to fix this because I haven't been able to reproduce this with a pure ARMI unit test.

[john-science]

So, the problem appears to be around here:

armi/armi/reactor/converters/geometryConverters.py

Line 1228 in 4fcea82

newAssem.makeUnique()

Essentially, if you run a growToFullCore() without a Case Settings file, it tries to rebuild the assemblies, and it appears to work, except it fails to name the assemblies correctly.

I have no found the bug yet. I am trying to unwind the 'load a core from a database' logical flow, to find where there is a misstep.

EDIT: And, of course, makeUnique() doesn't have any unit tests.

[john-science]

So, I have a solution!

https://github.com/john-science/armi/blob/d18bd7dc05de70a4484f0e374e7f0da99ae2043e/armi/bookkeeping/db/database3.py#L1104

The only problem I'm facing right now is: I can't generate a unit test that fails for this bug. So I can't really prove that my fix works. But the fix does work for certain trial databases users have given me.

[john-science]

It turns out the this fix caused some errors for a downstream project, and so I am reopening this ticket.

[jakehader]

It turns out the this fix caused some errors for a downstream project, and so I am reopening this ticket.

@john-science can you elaborate on the issue/bug now?

[jasonbmeng]

I believe this issue has been fixed via #615 and can be closed.

[john-science]

I believe this issue has been fixed via #615 and can be closed.

Oh has it? I thought this was still outstanding.

[john-science]

Based on a conversation with Jason, this ticket appears to be complete. Hurray!