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.

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

bad_alloc error when loading NMX nexus file #259

Open
jokasimr opened this issue Jan 30, 2025 · 8 comments · May be fixed by #262
Open

bad_alloc error when loading NMX nexus file #259

jokasimr opened this issue Jan 30, 2025 · 8 comments · May be fixed by #262
Assignees

Comments

@jokasimr
Copy link
Contributor

See https://git.esss.dk/dmsc-nightly/dmsc-nightly/-/jobs/53623

On scippnexus 24.11.1:

In [1]: import scippnexus as snx

In [2]: with snx.File('994160_00043336.hdf', "r", locking=False) as f:   dg = f[()]

MemoryError: std::bad_alloc
...

on scippnexus 24.3.1 this does not happen.

@jl-wynen
Copy link
Member

This is an NMX file, not DREAM.

The offending commit is 3a97343
The problem seems to be that in the new code in the commit, we have

group_dims = ('x_pixel_offset', 'y_pixel_offset', 'z_pixel_offset')
self._signal.shape = (1280, 1280, 8097)

which gets propagated to Field.__getitem__ for detector_number which then raises bad_alloc because the data volume is too large.

The detector_number in the file has shape (1280, 1280). And 8097 is the number of events, not a z-offset. Even though the shape matches the comment in the linked commit, this does not seem right to me. Why would we want to allocate such an array in the first place?

@jl-wynen jl-wynen changed the title bad_alloc error when loading Dream nexus file bad_alloc error when loading NMX nexus file Jan 30, 2025
@SimonHeybrock
Copy link
Member

I presume this is related to the NMX now having (or still not having?) an axis attribute for the pixel offset fields?

@jokasimr
Copy link
Contributor Author

I presume this is related to the NMX now having (or still not having?) an axis attribute for the pixel offset fields?

NMX files now have an axis attribute on the pixel offset fields

@jokasimr
Copy link
Contributor Author

And 8097 is the number of events, not a z-offset.

Looks like 8097 is the number of pulses in detector panel 0, not the number of events.
The number of events is considerably larger, the file size is 740MB.

The other panels have no pulses or events.

@SimonHeybrock
Copy link
Member

Maybe z_pixel_offset has a bad value in it's axis? If detector_number is 2-D but we have axis=2, ScippNexus might think it refers to some other data dimension?

@jokasimr jokasimr self-assigned this Jan 31, 2025
@jokasimr
Copy link
Contributor Author

Maybe z_pixel_offset has a bad value in it's axis? If detector_number is 2-D but we have axis=2, ScippNexus might think it refers to some other data dimension?

Yes this is definitely part of the issue.
z_pixel_offset has axis=3 (x=1, y=2).

But the length of z_pixel_offset is 1 so I don't know why it picks up 8097 as the size of that dimension.

@SimonHeybrock
Copy link
Member

I think it happens the other way round: This claims to be the third axis of the data/detector_number, therefore ScippNexus assigns it the dim label of the third axis.

@jokasimr
Copy link
Contributor Author

jokasimr commented Jan 31, 2025

Is it just a typo in the commit @jl-wynen referenced?

Should

                self._signal._grouping.sizes = dict(
                    zip(group_dims, self._signal.shape, strict=False)

be

                self._signal._grouping.sizes = dict(
                    zip(group_dims, self._signal._grouping.shape, strict=False)

?

@jokasimr jokasimr moved this from Selected to In progress in Development Board Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In progress
Development

Successfully merging a pull request may close this issue.

3 participants