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.

Sign up for GitHub

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

Jump to bottom

Fix ChunkV maintaining CurOffset when downsizing current chunk in All… #3769

Merged
merged 2 commits into from
Aug 18, 2023
Merged

Fix ChunkV maintaining CurOffset when downsizing current chunk in All… #3769

merged 2 commits into from
Aug 18, 2023

Conversation

pnorbert
Copy link
Contributor

…ocate() and creating the next chunk.

This fixes #3504

eisenhauer
eisenhauer previously approved these changes Aug 18, 2023
View reviewed changes
anagainaru
anagainaru previously approved these changes Aug 18, 2023
View reviewed changes
Copy link
Contributor

@anagainaru anagainaru left a comment

Choose a reason for hiding this comment

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

I tested it on mac and Summit and this fixes the bug. Except the small fix for window this looks good.

testing/adios2/unit/TestChunkV.cpp Outdated Show resolved Hide resolved
@pnorbert pnorbert dismissed stale reviews from anagainaru and eisenhauer via c54c379 August 18, 2023 11:13
@pnorbert pnorbert merged commit b5d08ca into ornladios:release_29 Aug 18, 2023
32 checks passed
@pnorbert pnorbert deleted the fix-chunkv-curoffset branch August 18, 2023 13:40
@pnorbert pnorbert mentioned this pull request Aug 18, 2023
Closed
@ax3l
Copy link
Contributor

ax3l commented Aug 18, 2023

Thank you all, that is awesome!

cc @vicentebolea
I think this might be worth patching into Spack and Conda-Forge releases of 2.9.1 for us...

@vicentebolea
Copy link
Collaborator

@ax3l a 2.9.2 is in horizon. We want to cut the 2.9.2 release as soon as #3459 is resolved.

This was referenced Aug 18, 2023
Merged
Merged
Merged
ax3l added a commit to ax3l/ADIOS2 that referenced this pull request Aug 20, 2023
@ax3l
Backport ChunkV Fix
1bfe1f3 
ornladios#3769
@ax3l ax3l mentioned this pull request Aug 20, 2023
Merged
ax3l added a commit to ax3l/ADIOS2 that referenced this pull request Aug 20, 2023
@ax3l
Backport ChunkV Fix
a4ba307 
ornladios#3769
@ax3l
Copy link
Contributor

ax3l commented Oct 26, 2023 •
edited
Loading

@vicentebolea thanks, I am validating #3459 - but since #3459 has a simple work-around for now, it is not a blocker for us for this release.

If for #3459 test coverage of

  • BP4 & BP5 reads
  • w/ and w/o compressors

are added to ADIOS2 CI then it is ready to go from my side 👍

@pnorbert pnorbert self-assigned this Jan 24, 2024
@pnorbert
Copy link
Contributor Author

This has not made it to master and hence 2.10rc1

@eisenhauer
Copy link
Member

This has not made it to master and hence 2.10rc1

Whoops...

@pnorbert
Copy link
Contributor Author

It does not fix #4008, unfortunately but still needed for the blosc2 issue

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

@eisenhauer eisenhauer eisenhauer approved these changes

@anagainaru anagainaru anagainaru left review comments

@vicentebolea vicentebolea Awaiting requested review from vicentebolea vicentebolea is a code owner

Assignees

@pnorbert pnorbert

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@pnorbert @ax3l @vicentebolea @eisenhauer @anagainaru