Explore: Block.createHomogenizedCopy should raise a NotImplementedError

[john-science]

In the class HexBlock we have a method createHomogenizedCopy() that works. But in the base class Block we have the same method, but all it does is return a copy of the block:

armi/armi/reactor/blocks.py

Lines 160 to 173 in 4f55d94

	def createHomogenizedCopy(self, pinSpatialLocators=False):
	"""
	Create a copy of a block.
	Notes
	-----
	Used to implement a copy function for specific block types that can
	be much faster than a deepcopy by glossing over details that may be
	unnecessary in certain contexts.
	This base class implementation is just a deepcopy of the block, in full detail
	(not homogenized).
	"""
	return copy.deepcopy(self)

This is obviously wrong, in that nothing was homogenized. But, from my perspective, this is dangerous as a user won't see that this is wrong and will just assume it worked.

I would like to explore changing this base class method to instead return a NotImplementedError.

[mgjarrett]

It's certainly a fair point that it's misleading for this to be called createHomogenizedCopy

This method was originally implemented to speed up the uniform mesh converter with HexBlocks by doing a lighter-weight copy than "deepcopy" (#1042). I had a good idea of what _createHomogenizedCopy should look like for a HexBlock, but I don't know the use cases for any other type of block well enough to have implemented a general method. So I just reverted back to a deepcopy for anything that wasn't a HexBlock. The goal was to keep the exact same functionality, but it does lead to confusing/misleading nomenclature. I didn't want to raise a NotImplementedError because that would be breaking something that had previously worked, in theory.

The other option would have been:

if isinstance(block, HexBlock):
    newBlock = block.createHomogenizedCopy()
else:
    newBlock = copy.deepcopy(block)

I was trying to avoid nesting that type check, but this really isn't so deep that it would be painful to type check every time.

---

BUT, I'd also like to reflect on #1042 with some hindsight:

There are a few reasons we might just want to remove createHomogenizedCopy.

1. It probably doesn't offer any practically significant speedup. When I first implemented it, the uniform mesh converter was taking an ungodly amount of time, so I was just trying to improve anything I could. Later on, I found that there was a function that was unnecessarily inside an extra layer of nesting, such that each block in an assembly would have its reaction rates calculated len(assembly) times. This double-nesting was accounting for the vast majority of the inefficiency.
2. Using block.setNumberDensities() in general is becoming increasingly more frowned-upon as we encounter several issues caused by it (e.g., [Suspect 🕵️‍♀️] - "De-homogenization" of Number Densities #1374 and several other issues in downstream applications). We should probably revert to a deepcopy of the block, and then set number densities by component:

newBlock = copy.deepcopy(sourceBlock)
compDensities = calculateComponentNumberDensities(sourceBlocks)
for c, compDensity in zip(newBlock, compDensities):
    c.setNumberDensities(compDensity)