Skip to content

Compatibility for new and old grid files#7

Merged
gauravharsha merged 6 commits intomainfrom
grids-update
Feb 16, 2026
Merged

Compatibility for new and old grid files#7
gauravharsha merged 6 commits intomainfrom
grids-update

Conversation

@gauravharsha
Copy link
Contributor

Add version checks for new grid files (pending the completion of green-grids PR 8):

  1. Both old and new grid files are allowed.
  2. Adds an attribute __grids_version__ to the output file
  3. In restart mode, if the version in grid_file isn't compatible with version in the restarting data file, then sc initialization throws appropriate error.

@gauravharsha
Copy link
Contributor Author

NOTE: tests will fail until the pull request for green-grids is merged.

@gauravharsha gauravharsha requested a review from egull February 12, 2026 08:17
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

Adds grid-file version tracking and restart-time compatibility checks to prevent mixing old/new green-grids formats, with accompanying test updates.

Changes:

  • Persist __grids_version__ into SC results files and validate version compatibility on restart.
  • Introduce a dedicated outdated_results_file_error exception type.
  • Extend SC tests/CMake to supply grid parameters and add restart/version mismatch scenarios.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.

File Description
src/green/sc/sc_loop.h Writes __grids_version__ to results and checks grids/results version compatibility during restart.
src/green/sc/except.h Adds outdated_results_file_error exception.
test/sc_test.cpp Updates tests to initialize grids/transformer usage and adds restart/version compatibility cases.
test/CMakeLists.txt Adds TEST_PATH compile definition to locate test data files.

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

Copy link
Contributor

@egull egull left a comment

Choose a reason for hiding this comment

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

Thanks! Have a look at the github comments but merge once you feel it's ready. Overall looks good!

@codecov-commenter
Copy link

codecov-commenter commented Feb 14, 2026

Codecov Report

❌ Patch coverage is 83.72093% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.65%. Comparing base (24e020f) to head (c6470a6).

Files with missing lines Patch % Lines
src/green/sc/common_defs.h 77.77% 4 Missing ⚠️
src/green/sc/sc_loop.h 87.50% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main       #7      +/-   ##
==========================================
- Coverage   96.45%   95.65%   -0.80%     
==========================================
  Files          10       10              
  Lines         648      690      +42     
==========================================
+ Hits          625      660      +35     
- Misses         23       30       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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


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

gauravharsha and others added 3 commits February 14, 2026 18:32
lingo in comments

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@gauravharsha
Copy link
Contributor Author

Copilot reviews addressed. Merging PR

@gauravharsha gauravharsha merged commit 565c4fc into main Feb 16, 2026
1 check passed
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.

4 participants