microscope-cockpit / cockpit

Cockpit is a microscope graphical user interface. It is a flexible and easy to extend platform aimed at life scientists using bespoke microscopes.
https://microscope-cockpit.org
GNU General Public License v3.0
37 stars 27 forks source link

MRC.py update problem. #896

Closed iandobbie closed 4 months ago

iandobbie commented 4 months ago

Davids recent changes to MRC.py, I think to help the transition to numpy2, have broken the image saving on windows in numpy2 but not numpy1.

To recretae this start cockpit in simulation mode (with no config files). Activate a camera and snap and image. Then right click on the image and choose save image. Then select a file name in the dialog and trigger the save.

On a windows machine (with numpy2) I get

 Traceback (most recent call last):
  File "C:\Users\idobbie1\src\cockpit\cockpit\gui\imageViewer\viewCanvas.py", line 427, in <lambda>
    self.Bind(wx.EVT_MENU, lambda event, action=action: action(), id=id)
  File "C:\Users\idobbie1\src\cockpit\cockpit\gui\imageViewer\viewCanvas.py", line 1036, in saveData
    cockpit.util.datadoc.writeDataAsMrcWithExthdr(self.imageData, path,
  File "C:\Users\idobbie1\src\cockpit\cockpit\util\datadoc.py", line 716, in writeDataAsMrcWithExthdr
    header = makeHeaderFor(data_out, XYSize = XYSize, ZSize = ZSize,
  File "C:\Users\idobbie1\src\cockpit\cockpit\util\datadoc.py", line 639, in makeHeaderFor
    header = makeHeaderForShape(data.shape, data.dtype.type, **kwargs)
  File "C:\Users\idobbie1\src\cockpit\cockpit\util\datadoc.py", line 666, in makeHeaderForShape
    Mrc.init_simple(header, Mrc.dtype2MrcMode(dtype),
  File "C:\Users\idobbie1\src\cockpit\cockpit\util\Mrc.py", line 962, in init_simple
    hdr.dvid= 0xc0a0
  File "C:\Users\idobbie1\src\cockpit\cockpit\util\Mrc.py", line 779, in __setattr__
    hdrArray[n][0] = v
 OverflowError: Python integer 49312 out of bounds for int16

But no such error with numpy1, it works fine.

On the mac matplotlib fails to import with numpy2 so I cant easily test this.

iandobbie commented 4 months ago

Ok, don't exactly know what I changed but managed to get the mac version running on numpy2 and found exactly the same error,

 Traceback (most recent call last):
  File “/Users/ID/src/cockpit/cockpit/gui/imageViewer/viewCanvas.py”, line 426, in <lambda>
    self.Bind(wx.EVT_MENU, lambda event, action=action: action(), id=id)
                                                        ^^^^^^^^
  File “/Users/ID/src/cockpit/cockpit/gui/imageViewer/viewCanvas.py”, line 1035, in saveData
    cockpit.util.datadoc.writeDataAsMrcWithExthdr(self.imageData, path,
  File “/Users/ID/src/cockpit/cockpit/util/datadoc.py”, line 713, in writeDataAsMrcWithExthdr
    header = makeHeaderFor(data_out, XYSize = XYSize, ZSize = ZSize,
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File “/Users/ID/src/cockpit/cockpit/util/datadoc.py”, line 636, in makeHeaderFor
    header = makeHeaderForShape(data.shape, data.dtype.type, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File “/Users/ID/src/cockpit/cockpit/util/datadoc.py”, line 663, in makeHeaderForShape
    Mrc.init_simple(header, Mrc.dtype2MrcMode(dtype),
  File “/Users/ID/src/cockpit/cockpit/util/Mrc.py”, line 964, in init_simple
    hdr.dvid= 0xc0a0
    ^^^^^^^^
  File “/Users/ID/src/cockpit/cockpit/util/Mrc.py”, line 781, in __setattr__
    hdrArray[n][0] = v
    ~~~~~~~~~~~^^^
 OverflowError: Python integer 49312 out of bounds for int16

So obviously numpy2 is defining different data sizes, or maybe an int16 rather than a uint16?

iandobbie commented 4 months ago

A couple of additional notes.

1) On numpy1 saving a single image with the snap followed by right click and save image works, and generates a valid image that can be reloaded.

2) On numpy2 the code is also broken on run experiment, so this is unable to work as it dies trying to do the file setup at the start of the experiment.

carandraug commented 4 months ago

This seems to be how numpy wraps around integers. And we're doing that for dvid and I'm not sure why.

Here's how to reproduce the issue:

>>> import numpy
>>> numpy.__version__
'2.0.0'
>>> import cockpit.util.Mrc
>>> hdr = cockpit.util.Mrc.makeHdrArray()
>>> hdr.dvid= 0xc0a0
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/carandraug/src/cockpit/cockpit/util/Mrc.py", line 781, in __setattr__
    hdrArray[n][0] = v
    ~~~~~~~~~~~^^^
OverflowError: Python integer 49312 out of bounds for int16
>>> import numpy
>>> numpy.__version__
'1.24.2'
>>> import cockpit.util.Mrc
>>> hdr = cockpit.util.Mrc.makeHdrArray()
>>> hdr.dvid= 0xc0a0
>>> hdr.dvid
-16224
carandraug commented 4 months ago

Numpy 1.24 deprecated "conversion of out-of-bound Python integers". Numpy 2 release notes also places this under the "deprecations" section but we're getting an error instead of a warning:

>>> import numpy as np
>>> np.int16(0xc0a0)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OverflowError: Python integer 49312 out of bounds for int16

Anyway, this was fixed in 3843ca55d6256b598a970b82d5c2d1bd70c25666 by simply assigning the value in decimal.

@iandobbie can you confirm that it all works now?

iandobbie commented 4 months ago

Tested on windows and macos with numpy 2 and allows saving and loading of MRC (.dv) files so closing as fixed.

Still don't understand why the value was defined in hex.

carandraug commented 4 months ago

Still don't understand why the value was defined in hex.

Yeah, same here. I checked the history and it's been that way since the start c5db11c3e. My guess is that it's the same in Priism.