Skip to content

Add option to ignore h5 context manager in preprocessing#1690

Open
mmccrackan wants to merge 8 commits into
masterfrom
260603_h5_ctx_2
Open

Add option to ignore h5 context manager in preprocessing#1690
mmccrackan wants to merge 8 commits into
masterfrom
260603_h5_ctx_2

Conversation

@mmccrackan

Copy link
Copy Markdown
Contributor

This allows preprocessing to bypass the h5 context manager when running off of site.

@tskisner

tskisner commented Jul 5, 2026

Copy link
Copy Markdown
Member

@mmccrackan, just checking- have you found any performance issue with the new temp file usage? Or is this bypass just to avoid the copy when you can already guarantee that only a single process is accessing an HDF5 file?

@mmccrackan

Copy link
Copy Markdown
Contributor Author

@mmccrackan, just checking- have you found any performance issue with the new temp file usage? Or is this bypass just to avoid the copy when you can already guarantee that only a single process is accessing an HDF5 file?

It seems when running SAT preprocessing, this incurs a big slowdown due to needing to copy both files simultaneously. I need to look into why it didn't seem to affect things when testing on LAT ASO data on Nersc, but I'm guessing it was just due to having less data overall + single node usage. We won't need it for running preprocessing off site so this was just to allow that. Longer term I think we should add an override_copy option to the H5ContextManager to ignore copying as that will be more general for off-site usage.

@tskisner

tskisner commented Jul 5, 2026

Copy link
Copy Markdown
Member

Ok, the main use case was long-running rsync at site during simultaneous appending to the files. Maybe there are other cases where h5 files are being appended while another process is reading, but I'm not sure. Agree about having a disable switch in the context manager.

@mmccrackan mmccrackan requested review from chervias and msilvafe July 10, 2026 14:15
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