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

Replace missing block for outside of sphere surface #15

Merged
merged 6 commits into from
Nov 7, 2024

Conversation

pshriwise
Copy link
Collaborator

This (re)adds support for the outer halfspace of sphere surfaces along with a new test to ensure it's working correctly.

Copy link
Contributor

@eepeterson eepeterson left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix on this @pshriwise but I think there are a couple things that need tweaking.

test/test_local.py Outdated Show resolved Hide resolved
test/test_local.py Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any subtract commands here since I don't think the test case was defined properly

middle_cell = openmc.Cell(region=-middle_sphere)
outer_cell = openmc.Cell(region=-outer_sphere)

g = openmc.Geometry([outer_cell, middle_cell, inner_cell])
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the order that the cells are listed in going to have an impact? I was expecting to see the outputs in the cubit journal file ordered by cell ID, but it looks reversed.

Copy link
Contributor

Choose a reason for hiding this comment

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

could also be worth tacking on a vacuum boundary condition to the outer sphere so that it makes a viable geometry that could ostensibly be run in openmc

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is the order that the cells are listed in going to have an impact? I was expecting to see the outputs in the cubit journal file ordered by cell ID, but it looks reversed.

I believe they'll be generated in the order they appear in the iterable passed to the Geometry object. Corresponding IDs on the volume objects in the resulting CAD would be great, but might be difficult to manage based on how Cubit handles ID spaces. An easier short-term solution would be to place each cell in it's own group with the ID in the name for reference to the original model.

could also be worth tacking on a vacuum boundary condition to the outer sphere so that it makes a viable geometry that could ostensibly be run in openmc

Agreed that it would be nice and I'm happy to include those changes here. Right now I'm planning on spending my time covering and verifying more geometry capabilities in the test suite (hopefully correctly in the future) than every test case being runnable in OpenMC.

Copy link
Contributor

Choose a reason for hiding this comment

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

super thanks!

pshriwise and others added 4 commits November 7, 2024 10:58
Co-authored-by: Ethan Peterson <eepeterson3@gmail.com>
Co-authored-by: Ethan Peterson <eepeterson3@gmail.com>
Copy link
Contributor

@eepeterson eepeterson left a comment

Choose a reason for hiding this comment

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

thanks for the fix @pshriwise!

@eepeterson eepeterson merged commit 70fea9a into openmc-dev:main Nov 7, 2024
1 check passed
@pshriwise pshriwise deleted the nested-spheres branch November 7, 2024 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants