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

Upgrade PM and MM #39

Merged
merged 22 commits into from
Aug 8, 2023
Merged

Upgrade PM and MM #39

merged 22 commits into from
Aug 8, 2023

Conversation

talonchandler
Copy link
Contributor

The current pycromanager==0.25.40 is not compatible with recOrder, so this PR suggests an upgrade to pycromanager==0.27.2. See mehta-lab/recOrder#373.

You might try upgrading pycromanager without upgrading MM, but I expect you'll need to upgrade MM to 2023-04-26 at the same time. I've updated the docs to reflect this, but feel free to change.

These upgrades should be tested on the scope before merging.

@ieivanov
Copy link
Collaborator

ieivanov commented Jun 2, 2023

Thanks @talonchandler, I think this will be good, we'll test and merge soon

@ieivanov
Copy link
Collaborator

ieivanov commented Jun 23, 2023

I find that when using pycromanager==0.27.2 ndtiff also needs to be pinned to 1.12.2. I recommend using MM nightly build 2023-03-12 rather than 2023-04-26 - I expect the pycromanager storage_monitor_fn will not work with 2023-04-26.

TODO: update the readme to reflect the correct MM version

@talonchandler
Copy link
Contributor Author

Sounds good, MM recommendation fixed.

Feel free to merge when you're ready.

@ieivanov
Copy link
Collaborator

I ran into a bug with the mantis acquisition that I'm trying to trace down before merging this PR. If the bug is in the java AnqEngJ then we'll need to upgrade to a newer PM version after I fix it.

@ieivanov
Copy link
Collaborator

We'll need to upgrade to the latest pycromanager and MM nightly build after micro-manager/AcqEngJ#94 and micro-manager/pycro-manager#623 get merged.

@ieivanov
Copy link
Collaborator

ieivanov commented Aug 6, 2023

@talonchandler @edyoshikun this PR is ready to merge, pending one more test on the microscope. Can I get a quick review of the code?

Copy link
Contributor

@edyoshikun edyoshikun left a comment

Choose a reason for hiding this comment

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

This LGTM. The most important change I think was the iohub to 0.1.0.dev4for making sure that the datasets created have the proper dtype and LUT when loaded into napari, and the pycromanager version which is the one on mantis at the moment. Thanks guys!

@ieivanov
Copy link
Collaborator

ieivanov commented Aug 7, 2023

Thanks! As far as the acquisition goes, there is no real dependency on iohub. For the next few acquisitions I will keep two conda environments and two MM versions, just in case we need a quick fallback.

@talonchandler
Copy link
Contributor Author

LGTM, thanks Ivan.

@ieivanov ieivanov linked an issue Aug 8, 2023 that may be closed by this pull request
@ieivanov
Copy link
Collaborator

ieivanov commented Aug 8, 2023

Before this PR, the mantis acquisition used MM nightly build 2023-02-08 with NDTiffStorage 2.10.0 for the LF acquisition and MM nightly build 2023-03-12 for the LS acquisition. A clone of the conda environment used before this PR is available as mantis_pm02540.

@ieivanov ieivanov merged commit 0d170ff into main Aug 8, 2023
2 checks passed
ieivanov added a commit that referenced this pull request Aug 8, 2023
@ieivanov ieivanov mentioned this pull request Aug 8, 2023
@ieivanov ieivanov deleted the upgrade-pm branch August 19, 2023 00:38
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.

Triggerscope not resetting after a failed acquisition
3 participants