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.

Sign up for GitHub

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

Jump to bottom

Zones overhaul #943

Merged
merged 29 commits into from
Oct 26, 2022
Merged

Zones overhaul #943

merged 29 commits into from
Oct 26, 2022

Conversation

joshuavictorchen
Copy link
Contributor

@joshuavictorchen joshuavictorchen commented Oct 17, 2022 •
edited
Loading

Description

Overhaul tooling in the reactor.zones module and remove application-specific zoning logic, which may be implemented as desired in downstream Interfaces.

This satisfies the main intent of #811. Enhancements may be added in the future.

Related issues:
#563
#800

Checklist

  • This PR has only one purpose or idea.
  • Tests have been added/updated to verify that the new/changed code works.
  • The release notes (location doc/release/0.X.rst) are up-to-date with any bug fixes or new features.
  • The documentation is still up-to-date in the doc folder.
  • The dependencies are still up-to-date in setup.py.

john-science and others added 25 commits August 3, 2022 13:37
@john-science
Removed all references to hot zones
3b014c9
@john-science
Minor cleanup
faeb477
@john-science
Removing byOrifice zone strategy
d4cac99
@john-science
Removing splitZones stuff
9705565
@john-science
Removed everyFA zoning strat
89a2d54
@john-science
Removing byFuelType zoning strat
6bd8e25
@john-science
Code cleanup
ba3f22c
@john-science
Removing byRing zoning strat
ee73f92
@john-science
Removing unused imports
e19b748
@john-science
Removing addRing - the Reactor might not have rings
34a166c
@john-science
Removing unused Zone.symmetry param
474d388
@john-science
Adding a type to the zone (Assem or Block)
f13f152
@john-science
First pass: complete rewrite of the Zone/Zones classes
b303bb4
@john-science
Fixing docstrings
55faaf0
@john-science
Working placeholder for zoning Strat plugin
2e5fd73
@john-science
ArmiPlugin.defineZoningStrat now in place
1b50679
@john-science
Testing defineZoningStrategy plugin hook
5fac427
@john-science
Change name to applyZoningStrategy
e890e8c
@john-science
Cleaning up Zones.summary
06b47c7
@joshuavictorchen
Adding removeXYZ functions to Zone and Zones classes in armi.reactor.…
c744f65 
…zones (#857)

Co-authored-by: Joshua Chen <jchen@terrapower.com>
@john-science
Merge commit '9a510ca' into zones_overhaul
b73f595
@joshuavictorchen
Zones overhaul updates (#938)
0dd52c9 
* remove buildZones function (in favor of using Interface for zoning)
* remove log re: zone creation from reactors.py (buildZones function was removed in prior commit)
* remove reactors.buildZones; move zones.buildManualZones to reactors module
* add unit test coverage, fix Zones.getAllLocations bug, move tests for buildManualZones to test_reactors
* update ThirdCoreHexToFullCoreChanger.convert to adjust zones when growing to full core
@joshuavictorchen
merge latest main into zones_overhaul
999cb3e
@joshuavictorchen
merge actual main tip into zones_overhaul
110301a
@joshuavictorchen
Merge branch 'main' into zones_overhaul
b1536f6
@joshuavictorchen joshuavictorchen marked this pull request as ready for review October 17, 2022 22:19
@joshuavictorchen joshuavictorchen marked this pull request as draft October 17, 2022 22:19
@john-science
Copy link
Member

john-science commented Oct 18, 2022 •
edited by joshuavictorchen
Loading

TODO:

  • We need to bump the ARMI version number.
  • We need to mention this in the release docs
  • In a future PR??? we should add some docs on how to define Zones.

@john-science
Copy link
Member

@sombrereau @drewejohnson

We are moving the super custom zones logic out of ARMI, so ARMI only provides a way to define Zones.

I assume this is better for both of your reactor projects, but I just want to ask you.

@john-science john-science added enhancement New feature or request architecture Issues related to big picture system architecture labels Oct 22, 2022
@john-science john-science marked this pull request as ready for review October 22, 2022 15:22
john-science
john-science approved these changes Oct 22, 2022
View reviewed changes
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.

@joshuavictorchen @opotowsky I think this is ready! Do you think the downstream repos are ready?

@joshuavictorchen
Copy link
Contributor Author

@joshuavictorchen @opotowsky I think this is ready! Do you think the downstream repos are ready?

Yep! Zoning board meeting has been scheduled for the downstream application/repos :)

@john-science john-science merged commit 47658eb into main Oct 26, 2022
@john-science john-science deleted the zones_overhaul branch October 26, 2022 17:09
scottyak pushed a commit to scottyak/armi that referenced this pull request Oct 27, 2022
@joshuavictorchen @scottyak
Overhauling ARMI Zones system (terrapower#943)
c96cd20 
Overhaul tooling in the reactor.zones module and remove application-specific zoning logic, which may be implemented as desired in downstream Interfaces.

This satisfies the main intent of terrapower#811. Enhancements may be added in the future.

Related issues:
terrapower#563
terrapower#800
@keckler keckler mentioned this pull request Oct 31, 2022
Closed
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@john-science john-science john-science approved these changes

Assignees
No one assigned
Labels
architecture Issues related to big picture system architecture enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@joshuavictorchen @john-science