Skip to content

Conversation

@The9Cat
Copy link
Member

@The9Cat The9Cat commented Dec 13, 2025

Summary

Check consistency of existing HDF5 before appending coefficients. This address issue #192.

Strategy

  • On write: Coefs::ExtendH5Coefs() calls the overridden member function CheckH5Params() to check whether the current coefficient dimensions, scale parameters, and force type is consistent with those in the file. Throws an exception on mismatch.
  • On read: the reader checks each stanza the expected dimensions based on the attributes and skips loading inconsistently ranked coefficient data

Notes

Compiles but only partly tested.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This pull request adds coefficient consistency checking for HDF5 files before appending new data, addressing issue #192. The implementation adds a new CheckH5Params() virtual method to the Coefs base class and implements it across all coefficient types to validate parameter consistency.

Key Changes:

  • Added CheckH5Params() method to validate coefficient dimensions, scale parameters, and force types before extending HDF5 files
  • Implemented shape validation during coefficient reading to skip inconsistently ranked data
  • Added Python bindings for the new CheckH5Params() method across all coefficient wrapper classes

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 24 comments.

File Description
expui/Coefficients.H Declares pure virtual CheckH5Params() method in base Coefs class and derived classes
expui/Coefficients.cc Implements CheckH5Params() for all coefficient types, adds parameter validation in ExtendH5Coefs(), and adds shape checking during coefficient reading
pyEXP/CoefWrappers.cc Adds pybind11 override declarations for CheckH5Params() method in all Python wrapper classes

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The9Cat and others added 3 commits December 13, 2025 13:33
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link

Copilot AI commented Dec 13, 2025

@The9Cat I've opened a new pull request, #195, to work on those changes. Once the pull request is ready, I'll request review from you.

The9Cat and others added 2 commits December 13, 2025 13:36
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link

Copilot AI commented Dec 13, 2025

@The9Cat I've opened a new pull request, #196, to work on those changes. Once the pull request is ready, I'll request review from you.

The9Cat and others added 13 commits December 13, 2025 13:36
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <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>
Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>
Co-authored-by: The9Cat <25960766+The9Cat@users.noreply.github.com>
[WIP] Address coefficient consistency check for HDF5 files
[WIP] Address feedback on coefficient consistency for HDF5 files
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Contributor

Copilot AI left a comment

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 3 out of 4 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The9Cat and others added 5 commits December 28, 2025 12:27
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.

2 participants