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.

Sign up for GitHub

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

Jump to bottom

Images being processed but not saved #682

Closed
carlkesselman opened this issue Sep 5, 2023 · 15 comments
Closed

Images being processed but not saved #682

carlkesselman opened this issue Sep 5, 2023 · 15 comments
Labels
bug Something isn't working

Comments

@carlkesselman
Copy link
Contributor

Bug report

The following test case seems to hang after saving 100 out of the 200 images in the acquisition. All of the images are processed, but onlly 100 are saved.

import os

import pycromanager
from pycromanager import multi_d_acquisition_events, Core, Acquisition
from spimmgr.config import java_loc, mm_app_path

config = r"C:\Program Files\Micro-Manager-2.0\MMConfig_demo.cfg"
pycromanager.start_headless(mm_app_path, config, java_loc=java_loc)

core = Core(debug=False)
print(f"Camera is {core.get_camera_device()}")

iprocess = 1
isaved = 1


def processed(image, metadata):
    global iprocess
    print(f"process image {iprocess}")
    iprocess += 1
    return image, metadata


def saved(_axis, _dataset):
    global isaved
    print(f"saved image {isaved}")
    isaved += 1


try:
    with Acquisition(directory="foo",
                     name="test",
                     image_saved_fn=saved,
                     image_process_fn=processed,
                     debug=False,
                     show_display=False) as mmacquisition:
        print(f"Start acquisition")
        mmacquisition.acquire(multi_d_acquisition_events(num_time_points=200))
    print(f"Acquisition complete")
    for p in pycromanager.acq_util.SUBPROCESSES:
        p.terminate()
except KeyboardInterrupt:
    print("Cores shut down.....")
    os._exit(1)

print("Cores shut down.....")
os._exit(1)

Version Info

  • Operating system: Windows 10
  • pycromanager version: 28.2
  • MicroManager version: 2.0.3 20230827
  • Python version: 3.11
  • Python environment (command line, IDE, Jupyter notebook, etc)
@carlkesselman carlkesselman added the bug Something isn't working label Sep 5, 2023
@carlkesselman
Copy link
Contributor Author

Did a little digging. The number 100 happens to be the size of the notification queue in Acquire class. Change to 150 and that is where it stalls out.....

@carlkesselman carlkesselman mentioned this issue Sep 5, 2023
Closed
@henrypinkard
Copy link
Member

Hopefully fixed by #685

I'm going to add a test for this now

Btw, I made some improvements to headless mode, so there is a now a stop_headless() so you shouldn't have to terminate manually.

@carlkesselman
Copy link
Contributor Author

carlkesselman commented Sep 7, 2023 via email

@henrypinkard
Copy link
Member

Great! No not yet. I'm making some examples/tests right now. Curious to hear what you think--changes can certainly be made if you have ideas for improvement.

@henrypinkard
Copy link
Member

There's also the python backend now: start_headless(python_backed=True). No file saving built in, so you have to use an image processor to divert images if you don't want to fill up RAM. Not sure if this fits your use case or not, but if you try it, let me know how it goes

@henrypinkard
Copy link
Member

You'll also need the latest CI build to test if you don't want to wait until tomorrows nightly build

@henrypinkard
Copy link
Member

Added test based on your example: #689

@henrypinkard
Copy link
Member

henrypinkard commented Sep 7, 2023 •
edited
Loading

Here's test code for receiving notifications:

from pycromanager import Acquisition, multi_d_acquisition_events, AcqNotification
import time
import numpy as np

events = multi_d_acquisition_events(num_time_points=10, time_interval_s=0)

print_notification_fn = lambda x: print(x.to_json())

start = time.time()
with Acquisition(directory='C:\\Users\\henry\\Desktop\\data', name='acq', notification_callback_fn=print_notification_fn,
                 show_display=False) as acq:
    start = time.time()
    future = acq.acquire(events)

    future.await_execution({'time': 5}, AcqNotification.Hardware.POST_HARDWARE)
    print('time point 5 post hardware')

    image = future.await_image_saved({'time': 5}, return_image=True)
    print('time point 5 image saved ', time.time() - start, '\t\tmean image value: ', np.mean(image))

    images = future.await_image_saved([{'time': 7}, {'time': 8}, {'time': 9}], return_image=True)
    assert (len(images) == 3)
    for image in images:
        print('mean of image in stack: ', np.mean(image))

# Make sure the returned images were the correct ones
on_disk = [acq.get_dataset().read_image(time=t) for t in [7, 8, 9]]
assert all([np.all(on_disk[i] == images[i]) for i in range(3)])

@carlkesselman
Copy link
Contributor Author

carlkesselman commented Sep 7, 2023 via email

@henrypinkard
Copy link
Member

So with the Python backend, the image_saved_fn callback will not be called and the acquisition engine will not write the ndtiff?

Right now, the image_saved_fn still gets called (when the image "saves" to RAM)--Im not sure I actually tested it though. No saving to NDTiff, as the writer code for this lives in Java. image_saved_fn and and image processors are pretty similar in python backend, with the difference being that the later allows you to prevent the image from being stored in memory in python (and accessible via a Dataset)

I like the idea of no Java (I never bothered to learn Java), but would like to make sure that I don’t end up an extra copy operation on the acquisition, which seems to be what happens when you define the callback with the java backend (looks like the java engine sends the image to python for processing by the callback, and then back again)

Yes exactly, there is an inefficient copy/serialization that should be avoidable with the python backend.

I made a python engine that is essentially a line-for-line copy of the Java one. So the nice thing about this is if you find bugs/improvements in the python engine, you can PR them and then it will be easy to improve the Java engine simultaneously.

So assuming I’m still using NDTiff, I would need to open the file and set the image_process_fn to do the write myself (rendering the directory argument useless?). I assume that I would save the image and then return None in the image argument from the callback?

Yes, with the python backend you should get an error if you pass directory. If you implement an image processor that returns None then you can save the images yourself. Or if you implement a an image_save_fn, the images will be stored in a Dataset in RAM

@carlkesselman
Copy link
Contributor Author

carlkesselman commented Sep 7, 2023 via email

@henrypinkard
Copy link
Member

Correct, there's no NDTiff writing from python. And this doesn't seem likely to be added anytime soon, because better to focus on new file saving in the c++ layer (micro-manager/mmCoreAndDevices#323), which would be faster than NDTiff and available from Java and Python.

As far as I know, Zarr and anything based on it currently lacks the features to be really fast. But I don't actually know what that the limit of its performance is.

So if you're saving to disk and NDTiff is good enough, probably best to stick with the Java backend. Though the python backend can still be a useful tool to swap in at times for debugging

@carlkesselman
Copy link
Contributor Author

carlkesselman commented Sep 7, 2023 via email

@carlkesselman
Copy link
Contributor Author

carlkesselman commented Sep 22, 2023 via email

@henrypinkard
Copy link
Member

Awesome!! Thanks for all your testing, suggestions, and PRs--they've really made pycromanager a lot better for everyone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@carlkesselman @henrypinkard