Skip to content

Conversation

@ThomasRyschawyEquinor
Copy link
Contributor

No description provided.

Copy link

@YeganehHallaj YeganehHallaj left a comment

Choose a reason for hiding this comment

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

Good job

# These 4 bytes indicate the data source for the SGZ file. Use 20 to indicate numpy.
buffer[76:80] = int_to_bytes(20)
if use_higher_samples_precision:
# higher precision minimum sample time/depth

Choose a reason for hiding this comment

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

I feel it is more readable if you define a function here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this method is only a couple of lines and I think it would actually make it harder to read if we define another function for this

Choose a reason for hiding this comment

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

I looked at the other parts of the function and it is not needed

if use_higher_samples_precision:
# higher precision minimum sample time/depth
buffer[84:92] = double_to_bytes(source.samples[0])
# higher precision sample interval (μs/m)

Choose a reason for hiding this comment

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

What would user expect when higher sample precision is enabled?
Should we add a comment specifying the data type and precision level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is defined in the SeismicZFP Header in the file-specification.md



def generate_fake_seismic(n_ilines, n_xlines, n_samples, min_iline=0, min_xline=0):
def generate_fake_seismic(n_ilines, n_xlines, n_samples, min_iline=0, min_xline=0, min_sample=0):

Choose a reason for hiding this comment

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

What is min_sample here?

Copy link
Contributor Author

@ThomasRyschawyEquinor ThomasRyschawyEquinor Nov 6, 2024

Choose a reason for hiding this comment

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

it's the minimum sample value for the z dimension of the cube. min_sample is the (missing) equivalent to the two other dimension of the cube, e.g. min_iline and min_xline. with min_sample you are able to specify where the cube starts in z dimension

Copy link

@YeganehHallaj YeganehHallaj left a comment

Choose a reason for hiding this comment

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

Looks good

@ThomasRyschawyEquinor ThomasRyschawyEquinor merged commit 56a2657 into master Nov 6, 2024
30 checks passed
@ThomasRyschawyEquinor ThomasRyschawyEquinor deleted the support-higher-stream-converter-samples-precision branch November 6, 2024 11:24
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