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

[Restructure/plasma] Refactor the simulation state initialization #2664

Merged
merged 62 commits into from
Jul 25, 2024
Merged

[Restructure/plasma] Refactor the simulation state initialization #2664

merged 62 commits into from
Jul 25, 2024

Conversation

wkerzendorf
Copy link
Member

@wkerzendorf wkerzendorf commented Jun 19, 2024 •
edited
Loading

Is based on the restructure radiation field PR. #2613

@wkerzendorf
change numberdensity to input
d583510
@wkerzendorf
fixed number density
7aaba5d
@wkerzendorf
some fixes
32b22ea
@wkerzendorf
removing density
261bbad
@wkerzendorf
remove atomic and isotope mass
df3c406
@wkerzendorf
add isotopic_number_density
d05e819
@wkerzendorf
Merge remote-tracking branch 'upstream/master' into restructure/plasm…
61f3db5 
…a_prepare
@wkerzendorf
add opacities package
e804942
@wkerzendorf
Update imports in property_collections.py, base.py, test_numba_interf…
e573f16 
…ace.py, transport_montecarlo_numba_interface.py, conftest.py, formal_integral.py, base.py, and macro_atom.py
@wkerzendorf
Add calculate_transition_probabilities function to util.py in macro_a…
73f2ddc 
…tom package
@wkerzendorf
Add calculate_transition_probabilities function to util.py in macro_a…
52d5523 
…tom package
@wkerzendorf
Remove unused imports and update plasma properties
9530fee
@wkerzendorf
add __init__ to macroatom
2fb7f92
@wkerzendorf
blackify tardis
6ac3e88
@wkerzendorf
blackified
801c92b
@wkerzendorf
chore: Update imports and remove unused code
ae0650d
@wkerzendorf
chore: Add PlanckRadiationField and DilutePlanckRadiationField classes
69dc1b5
@wkerzendorf
chore: Update imports and remove unused code
448d5c4
@wkerzendorf
removed density
c1dc52d
@wkerzendorf
Merge remote-tracking branch 'upstream/master' into restructure/plasm…
f053967 
…a_prepare
@wkerzendorf
Merge branch 'restructure/plasma_prepare' into restructure/plasma_opa…
17c033c 
…cities
@wkerzendorf
ruff output
5e02054
@wkerzendorf
cleanup and adding object mode
28c4348
@wkerzendorf
starting to make radiation_field a thing
cfffe5b
@wkerzendorf
Merge remote-tracking branch 'upstream/master' into restructure/plasm…
4e68723 
…a_opacities
@wkerzendorf
switched over to old tau_sobolev calculation
41779e2
@wkerzendorf
renamed function to indicate numba use
0d1bc4d
@wkerzendorf
Merge branch 'restructure/plasma_opacities' into restructure/plasma_r…
7cc84a2 
…adiation_field
@wkerzendorf
address comments
c924749
@wkerzendorf
Merge branch 'restructure/plasma_opacities' into restructure/plasma_r…
ad5a314 
…adiation_field
@andrewfullard
Copy link
Contributor

My previous comment still stands I think. This is also somewhat difficult to review given it is based on another PR.

@wkerzendorf
Copy link
Member Author

@andrewfullard - I presume you want me to move some of these functions to io - https://github.com/tardis-sn/tardis/pull/2664/files#diff-24577c067ab859927ea5939db7af1e47be15e9b6d48c73fc8a9177ce332b0dd1 ?

@andrewfullard
Copy link
Contributor

@andrewfullard - I presume you want me to move some of these functions to io - https://github.com/tardis-sn/tardis/pull/2664/files#diff-24577c067ab859927ea5939db7af1e47be15e9b6d48c73fc8a9177ce332b0dd1 ?

Yes please. That is what they are doing, after all- reading stuff in.

@wkerzendorf
Copy link
Member Author

@andrewfullard - moved

@andrewfullard
Copy link
Contributor

Looks good- is this ready to review?

@wkerzendorf
Copy link
Member Author

Looks good- is this ready to review?

I want to say yes

andrewfullard
andrewfullard previously approved these changes Jul 25, 2024
View reviewed changes
Copy link
Contributor

@andrewfullard andrewfullard left a comment

Choose a reason for hiding this comment

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

Looks good! Pending codestyle. I guess this makes Simulation approach the workflow in my branch.

Rodot-
Rodot- previously approved these changes Jul 25, 2024
View reviewed changes
Copy link
Contributor

@Rodot- Rodot- left a comment

Choose a reason for hiding this comment

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

Looks good to me

tardis/io/model/parse_packet_source_configuration.py
@@ -46,7 +46,7 @@ def initialize_packet_source(packet_source, config, geometry):
return packet_source


def parse_packet_source_from_config(config, geometry, legacy_mode_enabled):
def parse_packet_source_from_config(config, geometry, enable_legacy_mode):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why make this change? Don't we usually want booleans to be in a format that reads like flag is enabled or if flag_enabled?

@wkerzendorf wkerzendorf dismissed stale reviews from Rodot- and andrewfullard via 1355971 July 25, 2024 15:01
@wkerzendorf wkerzendorf merged commit 2766edd into tardis-sn:master Jul 25, 2024
14 of 15 checks passed
@wkerzendorf wkerzendorf deleted the restructure/simulation_from_config_cleanup branch July 25, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rodot- Rodot- Rodot- left review comments

@andrewfullard andrewfullard andrewfullard left review comments

Assignees
No one assigned
Labels
plasma restructure
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wkerzendorf @tardis-bot @andrewfullard @Rodot-