Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Photon Transport Simulation with Two Layers of Material & Unstructured Mesh #27
base: main
Are you sure you want to change the base?
Photon Transport Simulation with Two Layers of Material & Unstructured Mesh #27
Changes from 14 commits
23c1469
8d1f88b
d7e58a3
39e2d4b
3dfb567
5bb02ff
1fdb1b1
569eb28
1008270
19e86ef
6fa1367
c3cf5db
e217e00
94519f9
3854660
a3db102
4199c1b
fdcb11f
2e209a6
e8a305a
3aa85b3
1cff395
64d14d5
88a9e83
2259f79
84bac07
0713d73
5c03a21
d761b1c
fd3cb83
ed0e05d
c7a0c03
4568353
f91820f
2347332
d3b612e
99af15a
bc4e8d3
e06fba5
7afd568
2b5e67b
4fb3156
c81e21f
1ae47ce
c3509d6
6676191
e6c9518
3a09bf8
0d0418b
93d5b56
80382a4
ab1b09b
43270df
8aff672
0679a68
fc583b1
785511e
ee8df5b
790db58
3b808c1
052b312
771b147
7f76500
2a7d52c
6b18246
a3a9921
608adba
15f64ef
8af7f12
8aad344
3c272b8
95b52b1
2dde9f3
d07dd9d
da12a00
cc2dad7
589bff0
d813414
2ae754a
9e8bb76
d3bc003
18ca1b5
792717e
b1e219d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend putting this in a method that does everything you need for a single file. That kind of modularity enhances readability. You might also want to separate the functions of reading from a file, processing the data, and writing to files, so methods like
extract_source_data
,arrange_data
,write_source_density
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not clear that we need to import
openmc
here, if this method is only ever used when importing into another script that also importsopenmc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be missing something here, but OpenMC_PhotonTransport attempts to access openmc inside TwoLayers_Geometry even when only make_spherical_shells is imported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes multiple shells, so maybe change the name.
Could also generalize this to take an inner radius plus layers:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think materials will need to be an input as this function is also assigning materials to the geometry. I'm wondering if this can be done with one set of radii/thickness inputs and the materials from TwoLayers_Materials.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that the materials are necessary. My version had them bound in the
layers
. This kind of data structure can lead to clarity because the data gets bound together in instructive ways. An example of calling it might be:The function may be:
(There may be minor syntax errors, but this is the basic idea)