Skip to content

Reconcile cosmo-ics with dev - #534

Open
brantr wants to merge 49 commits into
devfrom
cosmo-ics
Open

brantr wants to merge 49 commits into
devfrom
cosmo-ics

Conversation

@brantr

@brantr brantr commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

The merge process with dev broke a few things with cosmo-ics, which this PR attempts to fix. This is a small, targeted PR meant to actually enable the cosmological ICs to run.

  • The behavior of the Runge-Kutta scheme changed. We've reinitialized a variable inside a loop and it seems to work.
  • A check in chemistry_functions.cpp now is triggered regarding how / whether H_fraction is set. I think there is an initialization step that must have been removed somehow. I've added this back.
  • There were two YHe declared in global.h.

Minor item:

  • Renamed cosmoics_seed to cosmo_ics_seed.

Output directory

  • The Parameter member P.outdir was removed and replaced with writer_manager.fname_template().nominal_output_dir_path().c_str(). This is not very user friendly, and seems to require the API to the initialization functions to be re-written if you want to write a file during initialization. Instead of making those more extensive changes, I added P.outdir back and then just copy the writer manager result into it. I don't seem the harm in this, so hopefully that is a minor addition that will pass muster.

@brantr

brantr commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

pre-commit.ci autofix

@mabruzzo

mabruzzo commented Sep 1, 2026 •

Copy link
Copy Markdown
Collaborator

FYI: This is on my radar.

I haven't reviewed it yet because I haven't fully thought through the best way for us to handle:

The Parameter member P.outdir was removed and replaced with writer_manager.fname_template().nominal_output_dir_path().c_str(). This is not very user friendly, and seems to require the API to the initialization functions to be re-written if you want to write a file during initialization. Instead of making those more extensive changes, I added P.outdir back and then just copy the writer manager result into it. I don't seem the harm in this, so hopefully that is a minor addition that will pass muster.

You are absolutely right that writer_manager.fname_template().nominal_output_dir_path().c_str() is not very user friendly. At the time that I refactored things, I suspect my thought process was that writing files at initialization was an unusual/atypical thing for us to do. Clearly, I was wrong.

I really don't want to track outdir in Parameters struct.1 Instead, I think the better solution is to add a method to WriterManager so we could write writer_manager.root_output_dir().2 But, I'm going to take another another day or so to mull things over.

In the particular instance where we are printing out the output directory during startup, I have an idea for more thoroughly printing out the simulation's runtime-configuration (but that's a topic for another time)

Footnotes

  1. As I've stated before, I think we should dispose of the Parameters struct. This is admittedly a somewhat slow and gradual process as we figure out the optimal home for certain quantities that must be accessed in various corners of the codebase (this was part of the motivation for introducing the model machinery). ↩

  2. Perhaps this is a sign that WriterManager should actually be called OutputManager. ↩

@brantr

brantr commented Sep 25, 2026

Copy link
Copy Markdown
Collaborator Author

@mabruzzo Following up on this item. Will you approve the kludge? If not, do you want to implement writer_manager.root_output_dir()?

@mabruzzo

Copy link
Copy Markdown
Collaborator

Yes. The plan is we're going to merge it and then I'm going to adjust it to a format I would prefer (i.e. store outdir as a std::string inside or Parameters)

This branch has not been deployed

No deployments
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