Ib ob build split - #34
Conversation
|
I kept a lot of the redundancies in the code by commenting them out. When I actually merge it, I would remove them all |
|
We don't need to create new pull requests every time we push updates to GitHub. Usually, we'd use the same PR for a specific new feature we're adding. When you push a branch to GitHub, the associated PR will automatically update |
connoramoreno
left a comment
connoramoreno
left a comment
There was a problem hiding this comment.
Pretty solid start on the IB/OB geometry overall. It would be nice to share an OpenMC slice plot to show the resulting geometry. I'll need to take a closer look at some point but here is some feedback to get you going.
|
|
||
| for thickness in range(10, 71, 10): #cm) |
There was a problem hiding this comment.
Let's not change this example too much. Let's just define a single thickness (rather than looping), and remove all the additional OpenMC stuff you added below
| # Outboard surfaces | ||
| minor_z_ob= self.minor_rad_z | ||
| minor_xy_ob= self.minor_rad_xy | ||
| surfaces["outboard"][surface] = openmc.ZTorus( | ||
| a=major_rad, | ||
| b=minor_z_ob, | ||
| c=minor_xy_ob | ||
| ) | ||
|
|
There was a problem hiding this comment.
I used the openmc.Ztorus(https://docs.openmc.org/en/latest/pythonapi/generated/openmc.ZTorus.html) for the separate donut shapes of different layers--First wall boundary, breeder, and shield.
There was a problem hiding this comment.
Outboard and inboard should be defined by the same surface
| b=minor_z_ob, | ||
| c=minor_xy_ob | ||
| ) | ||
| self.surfaces = surfaces |
There was a problem hiding this comment.
This overwrites the self.surfaces dictionary you defined in __init__. Either add your surfaces to that dictionary or don't instantiate a dictionary in __init__
| if layer_def is None: | ||
| continue |
There was a problem hiding this comment.
What is the purpose of this?
There was a problem hiding this comment.
About 8 lines below that code:
if "material_name" in layer_def:
material = self.get_material_by_name(
layer_def["material_name"]
)
For some reason, this was looping with None as well and gave me issues with the layer_def["material_name"].
This is kind of a stopgap measure to fix it.
gonuke
left a comment
gonuke
left a comment
There was a problem hiding this comment.
Looking forward to our discussion this morning.
In general, the best practice is to keep individual PRs as small as possible. There is no such thing as "too small".
There was a problem hiding this comment.
Agreed that this file should not need to be changed much or at all (depends a little on the input files syntax)
| # Outboard surfaces | ||
| minor_z_ob= self.minor_rad_z | ||
| minor_xy_ob= self.minor_rad_xy | ||
| surfaces["outboard"][surface] = openmc.ZTorus( | ||
| a=major_rad, | ||
| b=minor_z_ob, | ||
| c=minor_xy_ob | ||
| ) | ||
|
|
There was a problem hiding this comment.
Outboard and inboard should be defined by the same surface
3 participants
No description provided.