Encode ibm32 header fields on write - #376
Merged
BrianMichell merged 2 commits intoJun 16, 2026
Merged
Conversation
SegyFactory.create_traces did not IBM-encode ibm32 header fields, even though TraceAccessor IBM-decodes them on read. An ibm32 header written by the factory round-tripped to a wrong value: stored as a raw integer and read back through the IBM decode as garbage. Add a keyed to_ibm transform to the header pipeline before the byte-swap (the inverse of the read path's byte_swap then to_ieee), mirroring the data pipeline's existing ibm32 handling. Also promote ibm32 fields in create_trace_header_template to float32 so callers fill real floats that get encoded, matching the reader output and create_trace_sample_template. Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #376 +/- ##
==========================================
+ Coverage 95.86% 95.94% +0.07%
==========================================
Files 50 50
Lines 2663 2713 +50
Branches 149 151 +2
==========================================
+ Hits 2553 2603 +50
Misses 84 84
Partials 26 26 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Collaborator
I think it's appropriate for the bugfix to ensure we don't cause inadvertant trucation. |
BrianMichell
requested changes
Jun 16, 2026
BrianMichell
left a comment
Collaborator
There was a problem hiding this comment.
Please ensure that the codecov pipeline passes. I think applying that one change should satisfy it.
Contributor
Author
|
Thanks! The PR already has the float32 version, so nothing to change - good to merge whenever. |
Per review: drop the hand-rolled _retype_struct_fields helper and reuse the existing _modify_dtype_field from transforms.py to promote ibm32 header fields to float32. Removes the untested None-guard branch that left create_trace_header_template below patch coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
BrianMichell
approved these changes
Jun 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
SegyFactory.create_tracesdoes not IBM-encodeibm32header fields, even thoughTraceAccessorIBM-decodes them on read. The write path is therefore asymmetric with the read path: anibm32header written by the factory is stored as a raw integer, and reading it back through the IBM decode yields garbage.We hit this in a production MDIO->SEG-Y cut: an
ibm32header (azimuth) came out of the cut as a plain integer, which QC then flagged because the value no longer matched the source.Fix
create_traces: add a keyedto_ibmtransform to the header pipeline, placed before the byte-swap. This is the exact inverse of the read path (byte_swapthento_ieee) and mirrors the data pipeline's existingibm32handling.create_trace_header_template: exposeibm32fields asfloat32instead ofuint32, so callers fill real float values that then get encoded. This matches what the reader returns foribm32headers and whatcreate_trace_sample_templatealready does foribm32samples.The on-disk format is unchanged (4 bytes, IBM-encoded, big-endian); only the in-memory template dtype changes.
Open question
The
create_tracesencode change fixes the bug on its own, because the reader already hands callers afloat32array.The
create_trace_header_template->float32change is the consistency/usability part, and it's the only piece that changes a public dtype. Happy to drop it and keep the PR to just the encode if you'd rather avoid the template dtype change. Let me know which you prefer.Tests
TestSegyFactoryIbmHeader(big/little x 1/42 traces): on-disk bytes equalieee2ibm(value), plus a lossless round-trip.mypyandruffclean.