Skip to content

Update EXP IC routines for HDF5 output - #229

Open
The9Cat wants to merge 29 commits into
develfrom
generateHDF5ICs
Open

Update EXP IC routines for HDF5 output#229
The9Cat wants to merge 29 commits into
develfrom
generateHDF5ICs

Conversation

@The9Cat

@The9CatThe9Cat commented Aug 14, 2026

Copy link
Copy Markdown
Member

Summary

  • Add a new helper class to create particle ICs files using the new HDF5 schema
  • Updated gensph, gendisk, gendisk2d, zangics, cubeics, slabics with an -5,--hdf5 option for producing HDF5
  • The helper class makes these changes moderately non-invasive. None of the phase-space generation algorithms are changed by the call to write bodies.

This PR completes the deprecation of default ascii i/o for phase space, although ascii remains available if requested explicitly (e.g. OutAscii and automatic fallback to ascii phase-space input if HDF5 is not detected).

Why

  • HDF5 read/write is much more efficient on network file systems. E.g. ascii read write slows down ceph by more than an order of magnitude in write time.
  • Compressed, float32 saves approximately a factor of 5 in disk space relative to ascii. Float64 is an option but seems unnecessary for ICs.
  • Added compression to EmpCylSL cache files to squeeze out a bit more disk space. Level 5 yields a compression factor of about 1.5.

Tests

  • Added CTest tests to verify that the HDF5 files have the expected metadata and have particles masses that sum to some user-specificed expected value. Default is 1.
  • Tests passed for gensph, slabics and cubeics.
  • Test slabics IC generation
  • Perform a simulation from slabics generated ICs
  • Test gendisk IC generation
  • Perform a simulation from gendisk generated ICs

TBD

  • Set HDF5 output to the default after all tests are successfully completed.
  • Set the default EmpCylSL::H5compress=5. Was set to 0during testing.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds HDF5 output support to EXP initial-condition generators using the runtime particle schema.

Changes:

  • Introduces a shared HDF5 particle writer with MPI gathering, compression, and precision options.
  • Integrates HDF5 output across six IC generators.
  • Centralizes 1D mass models and adds HDF5 smoke tests.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 16 comments.

Show a summary per file
FileDescription
utils/ICs/ZangICs.ccAdds HDF5 output options.
utils/ICs/ParticleHDF5.HImplements the shared writer.
utils/ICs/massmodel1d.HRemoves the local model header.
utils/ICs/initial2d.ccAdds gendisk2d HDF5 output.
utils/ICs/initial.ccAdds gendisk HDF5 output.
utils/ICs/gensph.ccAdds distributed HDF5 output.
utils/ICs/genslab.ccReworks slab generation and output.
utils/ICs/cubeICs.ccAdds cube HDF5 output.
utils/ICs/CMakeLists.txtUses centralized mass models.
tests/CMakeLists.txtRegisters HDF5 smoke tests.
tests/check_hdf5_particles.pyValidates particle schemas.
include/massmodel1d.HCentralizes and expands 1D models.
exputil/massmodel1d.ccImplements centralized models.
exputil/CMakeLists.txtAdds models to exputil.
Suppressed comments (3)

exputil/massmodel1d.cc:375

  • setDF(..., nint) stores a requested quadrature size, but compute_model ignores it and always uses 400 points. Use the member so the public tuning parameter has an effect.
    exputil/massmodel1d.cc:4
  • This source still constructs std::ifstream at line 26 but no longer includes <fstream>. Compilation now depends on an unrelated transitive include from SLGridMP2.H; include the defining standard header directly.
    include/massmodel1d.H:706
  • The default constructor leaves both epsilon and dist_defined indeterminate, although distf immediately uses epsilon and callers inspect dist_defined. Initialize a deliberate regularization value and mark the completed model as defined, as the other constructors do.
 H = 1.0;
Omega = std::sqrt(2.0*M_PI/H);
norm = std::sqrt(2/Omega)/(M_PI*2.0*H);
Jmax = 0.5*Omega*H*H;
ModelID = "Uniform";

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threadutils/ICs/ParticleHDF5.H Outdated
Comment threadutils/ICs/ParticleHDF5.H Outdated
Comment threadutils/ICs/ParticleHDF5.H
Comment threadutils/ICs/genslab.cc
Comment threadutils/ICs/genslab.cc
Comment threadtests/check_hdf5_particles.py Outdated
Comment threadutils/ICs/initial.cc
Comment threadutils/ICs/initial2d.cc
Comment threadutils/ICs/ZangICs.cc
Comment threadutils/ICs/ParticleHDF5.H
The9Catand others added 6 commits August 14, 2026 16:36
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>
Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>
CopilotAIand others added 2 commits August 14, 2026 22:29
Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>
Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 16 out of 18 changed files in this pull request and generated 1 comment.

Suppressed comments (5)

exputil/massmodel1d.cc:2

  • This implementation still constructs std::ifstream (and std::ofstream in DEBUG builds), but the change removes <fstream> and none of the included project headers provides it. Add the direct include so exputil compiles.
    utils/ICs/initial.cc:1926
  • The gas HDF5 path, including creation of ngparam auxiliary datasets, remains untested: the added gendisk CTest does not pass --ngas and validates only halo and disk files. Add a small gas-enabled case and verify the gas file's count, mass, and auxiliary-field metadata.
 if (hdf5_output) {
if (hdf5_double)
EXP::ParticleHDF5::write<double>(gbods + ".h5", gas_particles, 0, ngparam, hdf5_filter);
else
EXP::ParticleHDF5::write<float>(gbods + ".h5", gas_particles, 0, ngparam, hdf5_filter);

utils/ICs/genslab.cc:67

  • This reintroduces HDF5 as the slabics default, although the PR description says HDF5 is opt-in and leaves changing the default as TBD. It also breaks existing invocations that expect slab.bods rather than slab.bods.h5; keep ASCII as the default and update the help strings accordingly.
 bool Mu, HDF5 = true;

include/massmodel1d.H:337

  • The default constructor computes rho0 from h before h is initialized, which is undefined behavior whenever Sech2u() is constructed. Initialize h first.
 rho0 = S0/(4.0*h);
h = dispz/(2.0*M_PI*S0);

include/massmodel1d.H:5

  • The new public header uses std::map, std::string_view, std::transform, std::tolower, std::ostringstream, std::invalid_argument, and other standard types without including their defining headers. In particular, the current include chain has no <map>, so translation units including this header fail to compile. Make the header self-contained.
#include <string>
#include <tuple>

Comment threadutils/ICs/initial.cc Outdated
Martin D. Weinbergand others added 2 commits August 16, 2026 12:16
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@The9Cat

Copy link
Copy Markdown
MemberAuthor

@michael-petersen: This PR that implements HDF5 phase-space input appears to be complete at this point. The changes are in exp with the one exception of the pyEXP.Basis.Cylindrical class which gets the new HDF5 compression options for EmpCylSL.

@The9CatThe9Cat added the enhancement New feature or request label Aug 16, 2026
@The9Cat

Copy link
Copy Markdown
MemberAuthor

@copilot : Can you change all of the defaults for IC generation to HDF5 rather than ascii? Also in IC generation routines that have H5compress=0 as the default, please change that to H5compress=5.

Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>

CopilotAI commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

@copilot : Can you change all of the defaults for IC generation to HDF5 rather than ascii? Also in IC generation routines that have H5compress=0 as the default, please change that to H5compress=5...

Done in commit Change IC generation defaults to HDF5 and H5compress=5. All six IC generators (gensph, cubeICs, ZangICs, gendisk, gendisk2d, slabics) now default to HDF5 output. Each has a new --ascii/-A flag to opt back to ASCII. gendisk's H5compress default is now 5. Test configs and cleanup lists updated to reference .h5 filenames.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@The9Cat@michael-petersen