Skip to content

Commit b1cf924

Browse files
committed
[fix] saves partial z-stack during mid error
Root cause of the original bug: a camera communication error could return an image with the wrong shape. On NumPy < 1.24, numpy.array() on mixed-shape arrays silently produces an object-dtype array, which cannot be written to TIFF (WriteDirectory() → AssertionError: 0).
1 parent 66819f8 commit b1cf924

5 files changed

Lines changed: 249 additions & 12 deletions

File tree

src/odemis/acq/acqmng.py

Lines changed: 50 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,7 @@ def run(self):
166166
"""
167167
remaining_t = self.estimate_total_duration()
168168
acquired_data = []
169+
exp = None
169170
# iterate through streams
170171
for stream in self._streams:
171172
zstack = []
@@ -201,6 +202,7 @@ def run(self):
201202

202203
else:
203204
# for each stream, iterate through zlevels
205+
z_exp = None # tracks failure within this stream's z-stack
204206
for i, z in enumerate(self._zlevels[stream]):
205207
# move the focuser
206208
self._actuator_f = stream.focuser.moveAbs({"z": z})
@@ -218,9 +220,10 @@ def run(self):
218220
try:
219221
# acquire this single stream, and get the data
220222
self._single_acqui_f = acquire([stream], self._settings_obs)
221-
data, exp = self._single_acqui_f.result()
222-
if exp:
223-
return acquired_data, exp
223+
data, z_exp = self._single_acqui_f.result()
224+
if z_exp:
225+
# exit the z-loop; partial z-stack will be assembled below
226+
break
224227
# check if cancellation happened while the acquiring future is working
225228
if self._future_state == CANCELLED:
226229
raise CancelledError()
@@ -234,20 +237,57 @@ def run(self):
234237
except CancelledError:
235238
raise
236239
except Exception as e:
237-
logging.exception("The acquisition failed at the %s-th zlevel of the stream %s, because %s" % (
238-
i + 1, stream, e))
239-
# TODO handle zstack assembling in case of error
240-
return acquired_data, e
240+
logging.exception("The acquisition failed at the %s-th zlevel of the stream %s",
241+
i + 1, stream.name.value)
242+
z_exp = e
243+
break # exit the z-loop; partial z-stack will be assembled below
241244

242245
# only if there is data acquired
243246
if data:
244-
zstack.append(data[0])
247+
da = data[0]
248+
# Validate spatial shape consistency across z-levels.
249+
# A camera communication error can return an image of the wrong size.
250+
# With NumPy < 1.24, numpy.array() on mixed-shape images silently
251+
# creates an object-dtype array, which cannot be written to TIFF.
252+
if zstack and da.shape[-2:] != zstack[0].shape[-2:]:
253+
logging.error(
254+
"Z-level %d of stream %s has shape %s, inconsistent with "
255+
"first z-level shape %s; stopping z-stack acquisition",
256+
i, stream.name.value, da.shape[-2:], zstack[0].shape[-2:]
257+
)
258+
z_exp = ValueError(
259+
"Z-level %d image shape %s is inconsistent with first "
260+
"z-level shape %s" % (i, da.shape[-2:], zstack[0].shape[-2:])
261+
)
262+
break
263+
zstack.append(da)
264+
245265
# update the remaining time
246266
remaining_t -= stream.estimateAcquisitionTime()
247267
self._main_future.set_end_time(time.time() + remaining_t)
248268

249-
zcube = assembleZCube(zstack, self._zlevels[stream])
250-
acquired_data.append(zcube)
269+
# Assemble whatever z-levels were acquired (partial or complete).
270+
# This also handles the TODO: save partial z-stack data on failure.
271+
if zstack:
272+
if len(zstack) < len(self._zlevels[stream]):
273+
logging.warning(
274+
"Partial z-stack for stream %s: assembling %d of %d levels",
275+
stream.name.value, len(zstack), len(self._zlevels[stream])
276+
)
277+
try:
278+
zcube = assembleZCube(zstack, self._zlevels[stream][:len(zstack)])
279+
acquired_data.append(zcube)
280+
except Exception as e:
281+
logging.exception("Failed to assemble z-stack for stream %s",
282+
stream.name.value)
283+
if z_exp is None:
284+
z_exp = e
285+
elif z_exp is None:
286+
logging.warning("All z-levels for stream %s returned empty data, skipping",
287+
stream.name.value)
288+
289+
if z_exp is not None:
290+
return acquired_data, z_exp
251291

252292
# state that the future has finished
253293
with self._future_lock:

src/odemis/acq/test/acq_test.py

Lines changed: 151 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import os
2424
import time
2525
import unittest
26+
from concurrent.futures import Future
2627
from concurrent.futures._base import CancelledError
2728
from unittest import mock
2829

@@ -33,13 +34,14 @@
3334
import odemis.acq.stream as stream
3435
from odemis import model
3536
from odemis.acq import acqmng
36-
from odemis.acq.acqmng import SettingsObserver, acquireZStack
37+
from odemis.acq.acqmng import SettingsObserver, ZStackAcquisitionTask, acquireZStack
3738
from odemis.acq.leech import ProbeCurrentAcquirer
3839
from odemis.acq.move import MicroscopePostureManager, FM_IMAGING, SEM_IMAGING, LOADING
3940
from odemis.driver import xt_client
4041
from odemis.driver.test.xt_client_test import CONFIG_FIB_SEM, CONFIG_FIB_SCANNER, CONFIG_DETECTOR
4142
from odemis.util import testing
4243
from odemis.util.comp import generate_zlevels
44+
from odemis.dataio import tiff
4345

4446
logging.getLogger().setLevel(logging.DEBUG)
4547

@@ -856,5 +858,153 @@ def test_settings_observer_metadata_with_zstack(self):
856858
self.assertEqual(data[0].metadata[model.MD_EXTRA_SETTINGS]
857859
["Camera"]["exposureTime"], [0.023, "s"])
858860

861+
862+
def _make_sim_future(result=None):
863+
"""
864+
Return a stdlib Future already completed with result.
865+
866+
:param result: value to store in the future
867+
:return: completed concurrent.futures.Future
868+
"""
869+
f = Future()
870+
f.set_result(result)
871+
return f
872+
873+
874+
def _make_sim_data_array(shape=(64, 64), dtype=numpy.uint16):
875+
"""
876+
Return a minimal 2-D DataArray suitable as a z-level image.
877+
878+
:param shape: 2-tuple (height, width)
879+
:param dtype: NumPy dtype for the pixel data
880+
:return: model.DataArray with pixel-size and position metadata
881+
"""
882+
md = {
883+
model.MD_DIMS: "YX",
884+
model.MD_PIXEL_SIZE: (1e-7, 1e-7),
885+
model.MD_POS: (0.0, 0.0),
886+
}
887+
return model.DataArray(numpy.zeros(shape, dtype=dtype), md)
888+
889+
890+
def _make_sim_stream(name="mock_stream"):
891+
"""
892+
Build a MagicMock that satisfies the interface used by ZStackAcquisitionTask.
893+
894+
:param name: human-readable name for the stream mock
895+
:return: unittest.mock.MagicMock mimicking a Stream
896+
"""
897+
s = mock.MagicMock()
898+
s.name.value = name
899+
s.estimateAcquisitionTime.return_value = 0.0
900+
s.focuser.moveAbs.return_value = _make_sim_future(None)
901+
return s
902+
903+
904+
def _make_sim_task(stream_mock, zlevels):
905+
"""
906+
Construct a ZStackAcquisitionTask with a mock ProgressiveFuture.
907+
908+
Both guessActuatorMoveDuration (called in __init__) and
909+
estimate_total_duration (called inside run()) are patched to avoid
910+
the need for real actuator hardware.
911+
912+
:param stream_mock: mock Stream object
913+
:param zlevels: dict mapping stream_mock to list of z positions
914+
:return: (task, mock_future) tuple ready to call task.run() on
915+
"""
916+
future = mock.MagicMock()
917+
with mock.patch("odemis.acq.acqmng.guessActuatorMoveDuration", return_value=0.0):
918+
task = ZStackAcquisitionTask(future, [stream_mock], zlevels, settings_obs=None)
919+
task.estimate_total_duration = mock.MagicMock(return_value=1.0)
920+
return task, future
921+
922+
923+
class TestZStackPartialFailureSim(unittest.TestCase):
924+
"""
925+
Simulation tests (no hardware) for the fix that saves partial z-stack data
926+
when a camera error occurs during an acquisition.
927+
928+
Root cause of the original bug: a camera communication error could return
929+
an image with the wrong shape. On NumPy < 1.24, numpy.array() on
930+
mixed-shape arrays silently produces an object-dtype array, which cannot
931+
be written to TIFF (WriteDirectory() → AssertionError: 0).
932+
933+
Shape validation in assembleZCube() and ZStackAcquisitionTask.run(),
934+
plus partial z-stack assembly is implemented instead of discarding data on failure.
935+
"""
936+
937+
def test_full_success_returns_zcube(self):
938+
"""
939+
When all z-levels succeed, run() returns a single ZYX DataArray and no exception.
940+
"""
941+
n = 3
942+
zlevels_list = [i * 1e-6 for i in range(n)]
943+
s = _make_sim_stream("fluo")
944+
task, _ = _make_sim_task(s, {s: zlevels_list})
945+
946+
good_img = _make_sim_data_array((64, 64))
947+
acq_futures = [_make_sim_future(([good_img], None)) for _ in range(n)]
948+
949+
with mock.patch("odemis.acq.acqmng.acquire", side_effect=acq_futures):
950+
data, exp = task.run()
951+
952+
self.assertIsNone(exp)
953+
self.assertEqual(len(data), 1)
954+
self.assertEqual(data[0].shape, (n, 64, 64))
955+
self.assertNotEqual(data[0].dtype, object)
956+
957+
def test_wrong_shape_mid_zstack_saves_partial(self):
958+
"""
959+
When a z-level image has a wrong spatial shape (truncated camera read),
960+
run() must stop, assemble only the valid z-levels, and report the error.
961+
962+
This is the primary regression test for the TIFF-crash bug.
963+
"""
964+
zlevels_list = [0.0e-6, 1.0e-6, 2.0e-6]
965+
s = _make_sim_stream("fluo")
966+
task, _ = _make_sim_task(s, {s: zlevels_list})
967+
968+
good_img = _make_sim_data_array((64, 64))
969+
bad_img = _make_sim_data_array((32, 64)) # truncated height — simulates camera error
970+
971+
acq_futures = [
972+
_make_sim_future(([good_img], None)),
973+
_make_sim_future(([good_img], None)),
974+
_make_sim_future(([bad_img], None)), # 3rd level: wrong shape
975+
]
976+
977+
with mock.patch("odemis.acq.acqmng.acquire", side_effect=acq_futures):
978+
data, exp = task.run()
979+
980+
# Partial data must be saved
981+
self.assertEqual(len(data), 1)
982+
zcube = data[0]
983+
# Must NOT be an object-dtype array (the original bug)
984+
self.assertNotEqual(zcube.dtype, object)
985+
# Only the 2 valid levels are included
986+
self.assertEqual(zcube.shape, (2, 64, 64))
987+
# Error must be reported
988+
self.assertIsNotNone(exp)
989+
990+
def test_first_zlevel_fails_returns_empty_data(self):
991+
"""
992+
When the very first z-level fails, no z-cube can be assembled.
993+
run() must return an empty data list and the exception.
994+
"""
995+
zlevels_list = [0.0e-6, 1.0e-6]
996+
s = _make_sim_stream("fluo")
997+
task, _ = _make_sim_task(s, {s: zlevels_list})
998+
999+
hw_error = IOError("Camera connection lost")
1000+
1001+
with mock.patch("odemis.acq.acqmng.acquire",
1002+
return_value=_make_sim_future(([], hw_error))):
1003+
data, exp = task.run()
1004+
1005+
self.assertEqual(len(data), 0)
1006+
self.assertIs(exp, hw_error)
1007+
1008+
8591009
if __name__ == "__main__":
8601010
unittest.main()

src/odemis/gui/cont/acquisition/cryo_acq.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -536,9 +536,14 @@ def _on_export_data_done(self, future):
536536
"""
537537
Called after exporting the data
538538
"""
539+
try:
540+
data = future.result()
541+
except Exception:
542+
logging.exception("Failed to save acquired data to file")
543+
self._reset_acquisition_gui(text="Failed to save data (see log panel).", state=ST_FAILED)
544+
return
539545
self._reset_acquisition_gui(state=ST_FINISHED)
540546
self._update_acquisition_time()
541-
data = future.result()
542547
self._display_acquired_data(data)
543548

544549
def _create_cryo_filename(self, filename: str, acq_type: Optional[str] = None) -> str:

src/odemis/util/img.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,7 +1286,22 @@ def assembleZCube(images, zlevels):
12861286
:param images: (list of DataArray of shape YX) list of z ordered images
12871287
:param zlevels: (list of float) list of focus positions
12881288
:return: (DataArray of shape ZYX) the data array of the xyz cube
1289+
:raises ValueError: if images is empty or the images have inconsistent spatial shapes
12891290
"""
1291+
if not images:
1292+
raise ValueError("Cannot assemble z-cube from an empty image list")
1293+
1294+
# Validate that all images have the same spatial (YX) dimensions.
1295+
# With NumPy < 1.24, numpy.array() on a list of arrays with different shapes
1296+
# silently creates an object-dtype array, which cannot be written to TIFF.
1297+
ref_shape = images[0].shape[-2:]
1298+
for idx, im in enumerate(images[1:], start=1):
1299+
if im.shape[-2:] != ref_shape:
1300+
raise ValueError(
1301+
"Z-stack image %d has shape %s, inconsistent with first image shape %s"
1302+
% (idx, im.shape[-2:], ref_shape)
1303+
)
1304+
12901305
# images is a list of 3 dim data arrays.
12911306
# Will fail on purpose if the images contain more than 2 dimensions
12921307
ret = numpy.array([im.reshape(im.shape[-2:]) for im in images])

src/odemis/util/test/img_test.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2068,6 +2068,33 @@ def test_assemble_zcube_z_order(self):
20682068
self.assertGreater(output_rev_z.metadata[model.MD_PIXEL_SIZE][2], 0)
20692069
numpy.testing.assert_array_equal(output_da_after, output_rev_z)
20702070

2071+
def test_assemble_zcube_empty_list_raises(self):
2072+
"""
2073+
assembleZCube() must raise ValueError when given an empty image list.
2074+
"""
2075+
with self.assertRaises(ValueError):
2076+
img.assembleZCube([], [])
2077+
2078+
def test_assemble_zcube_inconsistent_shapes_raises(self):
2079+
"""
2080+
assembleZCube() must raise ValueError when z-level images have different shapes.
2081+
2082+
On NumPy < 1.24, numpy.array() on mixed-shape arrays silently creates an
2083+
object-dtype array, which cannot be written to TIFF. The fix detects this
2084+
early and raises explicitly.
2085+
"""
2086+
images = [
2087+
model.DataArray(numpy.zeros(self.size, dtype=numpy.uint16), self.md),
2088+
model.DataArray(numpy.zeros(self.size, dtype=numpy.uint16), self.md),
2089+
model.DataArray(numpy.zeros((self.size[0] // 2, self.size[1]), dtype=numpy.uint16), self.md),
2090+
]
2091+
zlevels = self.z_list[:3]
2092+
2093+
with self.assertRaises(ValueError) as ctx:
2094+
img.assembleZCube(images, zlevels)
2095+
2096+
self.assertIn("shape", str(ctx.exception).lower())
2097+
20712098

20722099
class TestFloodFill(unittest.TestCase):
20732100

0 commit comments

Comments
 (0)