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

Give all ArmiObjects shape; change Blocks and Components to have shapes rather than be shapes #540

Open
ntouran opened this issue Jan 18, 2022 · 4 comments
Assignees
Labels
architecture Issues related to big picture system architecture complex Expected to be a complex issue

Comments

@ntouran
Copy link
Member

ntouran commented Jan 18, 2022

In discussion in #503, we came up with a need for a Composite containing a blend of a graphite matrix with a number of TRISO ComponentGroups. In order to do this, we need Composite to be able to have a shape. However, at the moment, only Blocks and Components have the concept of shape.

Thus, we think we need to refactor the reactor model to allow all ArmiObjects to have a .shape attribute that provides dimensional information rather than getting this shape information from the Block subclasses (e.g. HexBlock) and the Component subclasses (e.g. Circle).

This will allow for more flexible model generation and code reuse.

I think we can do this major refactor without changing any behavior of the code, and also without changing input.

It will basically involve:

  • make a shapes subpackage in reactor
  • Make a shape class for all shape-based component subclasses.
    • We may not be able to remove the shape-based Component subclasses themselves, since the dimensional params are defined there. Though we could infer the params from the underlying shape. But then the DB model would need to be changed, which is a much bigger lift. DB stores data for each shape subclass individually :o
  • Add a .shape attribute to ArmiObject
  • Change all code that gets dimensions of blocks to use its new .shape object
  • Change all code that gets dimensions of components to use its new .shape object

After this is done, then we can go back into #525 and allow the creation of a true TRISO fuel compact right in the block input, and related.

I think the hardest part is figuring out how to have the DB store the dimensional dimensional params for this ArmiObject.

@ntouran ntouran added architecture Issues related to big picture system architecture complex Expected to be a complex issue labels Jan 18, 2022
@ntouran ntouran self-assigned this Jan 18, 2022
@john-science
Copy link
Member

I like the idea! Do you have any guesses on what current tools adding a .shape would break?

I'm just asking for documentation purposes.

So we can build a LOE for this.

@ntouran
Copy link
Member Author

ntouran commented May 3, 2022

Do you have any guesses on what current tools adding a .shape would break?

I am hoping we can do this without breaking the API at all, so hopefully we won't break any existing plugins. The biggest threat is that we might have to up-rev the ARMI database, which makes loading old cases from newer code harder.

@ntouran
Copy link
Member Author

ntouran commented May 3, 2022

Later hesitations: In thinking harder about this and the way the Database stores things, it may end up being a simpler implementation to allow Components to have children (😲). The DB layout field can facilitate this as is, and the storage of different shaped components is already in place as well. Taking that approach would be a different ticket, obviously.

@john-science
Copy link
Member

@ntouran This ticket is still assigned to you. That's cool with me, but if you aren't planning on working on it, please un-assign it.

(You are not being singled out, I am going through all the ARMI tickets.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architecture Issues related to big picture system architecture complex Expected to be a complex issue
Projects
None yet
Development

No branches or pull requests

2 participants