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

Add component group capabilities #525

Merged
merged 1 commit into from
Jan 20, 2022
Merged

Conversation

ntouran
Copy link
Member

@ntouran ntouran commented Jan 6, 2022

Description

This just allows the user input of component groups intended to mix
different free components in different fractions. As is, this only
allows the input but does not implement any actual construction of
objects. It is a follow-up to #505 as part of the larger #504 task
related to improving input flexibility.

This needs to have a test that shows it reading/writing to/from
the database.


Checklist

  • Tests have been added/updated to verify that the new or changed code works.
  • Docstrings were included and updated as necessary
  • The code is understandable and maintainable to people beyond the author
  • There is no commented out code in this PR.

If user exposed functionality was added/changed:

  • Documentation added/updated in the doc folder.
  • New or updated dependencies have been added to setup.py.

@ntouran ntouran added the architecture Issues related to big picture system architecture label Jan 6, 2022
@ntouran ntouran requested a review from john-science January 6, 2022 00:40
Copy link
Member

@john-science john-science left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm okay approving this as long as we keep working on the new components-with-components feature set. It's a good stepping stone.

@ntouran ntouran requested a review from john-science January 7, 2022 18:15
@ntouran ntouran changed the title Add component group definitions to input. Add component group capabilities Jan 7, 2022
@@ -204,10 +204,6 @@ def test_fromComponent(self):
class TestShapedComponent(TestGeneralComponents):
"""Abstract class for all shaped components"""

def test_getBoundingCircleOuterDiameter(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we removing this test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because we implemented it! but you're right, we should just update the test to actually test it, eh?

@john-science
Copy link
Member

Hey Nick. Can you address the two conversations above?

I just want to merge this PR, and it looks like they will both be fast.

This allows the user to input component groups intended to mix different
free components in different fractions. It is a follow-up to terrapower#505 as
part of the larger terrapower#504 task related to improving input flexibility.

This implements the addition of groups of components to blocks, which
can be used to create more complex and flexible models.

The code changes solved a few residual issues where iterating over the
children of a block were assuming all children would be Components. Now
they can be Composites with multiple Components or Components.

Some upgrades to DerivedShapes were necessary so that they could work
with volumetric components in addition to pure 2D ones.

These are additional steps toward terrapower#504, but true practical usage of
these groups is not quite done yet. More followups are needed.

Added a new test covering negative volume in derived shapes
@ntouran ntouran merged commit e2a064c into terrapower:master Jan 20, 2022
@ntouran ntouran deleted the componentgroups branch January 20, 2022 17:46
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants