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

Migrate Parameter Parsing: moved configuration of EnzoInitiailFeedbackTest into constructor #378

Merged

Conversation

mabruzzo
Copy link
Contributor

Pull request summary

Followup from #349: moved parameter parsing for EnzoInitiailFeedbackTest from EnzoConfig into constructor.

Aside: This sort of migration is the sort of thing that needs to be done for a lot more examples. This is a great first-time-contribution for new developers. I'm going to create an Issue summarizing the process and that links to this PR as an example (in reality, I will probably link to the particular commit that makes this change).

@mabruzzo mabruzzo force-pushed the param-prototype-initialfeedback-test branch from d1021d1 to 222bf70 Compare April 3, 2024 20:30
@mabruzzo mabruzzo mentioned this pull request Apr 18, 2024
22 tasks
Copy link
Contributor

@clairekope clairekope left a comment

Choose a reason for hiding this comment

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

One question following up on a code comment, but this has nothing to do with the functionality of the PR

Comment on lines +167 to 168
// shouldn't we be using a self-consistent mmw?
const enzo_float mol_weight = enzo::fluid_props()->mol_weight();
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume this is reading the molecular weight from somewhere else, like another parameter? Maybe make this an issue so we don't loose track of it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, users have the option to specify a single,fixed mmw weight by using the parameter Physics:fluid_props:mol_weight. I added the comment because we go on and subsequently use various species fields.

It doesn't appear that the mmw computed from the species fields will be consistent with the mmw specified in the parameters.

(I'll add this note to the species issue)

@jwise77 jwise77 merged commit d4300b6 into enzo-project:main May 13, 2024
7 checks passed
@mabruzzo mabruzzo deleted the param-prototype-initialfeedback-test branch May 14, 2024 17:19
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.

3 participants