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

PyDevice PR #414

Merged
merged 237 commits into from
Jun 11, 2024
Merged

PyDevice PR #414

merged 237 commits into from
Jun 11, 2024

Conversation

JeroenDoornbos
Copy link
Contributor

PyDevice is an adapter that imports objects from a Python script and integrates them into MicroManager as devices (e.g. a camera or a stage). This integration enables the use of Python scripts to control microscope hardware, without requiring any programming experience from the end user. Essentially, PyDevice acts as a translator, allowing Python-developed objects to be used in MicroManager with almost no interfacing code required.

We have spent a while working with it and integrating it with our OpenWFS framework. The idea is that both developers and users can have the control they require, without sacrificing modularity and development speed. The OpenWFS repo: https://github.com/IvoVellekoop/openwfs also contains some more advanced devices, such as a laser-scanning object that functions as a MicroManager camera.

We recommend the README.md or example_devices/PyDevice.md as starting points.

It was built by Ivo Vellekoop and Jeroen Doornbos, please let us know what you think! We are very interested in your insights.

@IvoVellekoop
Copy link
Contributor

@tlambert03 Thank you for the suggestion. I've added support for annotated_types so that you can use Ge, Le, Interval and Unit.
The modules astropy.units, numpy and annotated_types are all completely optional now, so PyDevice also works with a vanilla Python installation. Could you perhaps have a look at the new bootstrap.py? I'd love to hear your thoughts. Implementing pint support would be a nice and simple extension.

@henrypinkard Thanks for all the feedback. I fixed most of the issues:

  • IsExposureSequenceable should return false. ✓
  • Moving pydevice.md from the examples folder. This was actually an obsolete file. I removed it. All documentation is now in the README.md. ✓
  • file not found. The file variable is now set to 'pydevice' when running from Micro-Manager. ✓
  • Complicated hierarchy for finding which python interpreter to use. This was simplified a lot when implementing the DELAYLOAD functionality. It is now also documented clearly (I hope) in README.md.
  • The camera.py file provided in the examples does not work. Fixed, and I verified that all other examples also load correctly. ✓

Remaining issues that we should think about a bit more:

  • If you change the ScriptPath in the hardware config without adding/deleting the hub device, it doesn't seem to update what peripheral devices are available (its possible this is a bug on the MM side)

If the Hub object is destroyed and recreated, it should load the new Python script. If this doesn't happen, indeed we need to investigate what is happening exactly.

  • The current setup involves creating a dictionary of devices in Python, which adds an extra layer of configuration outside of MM. It might be more robust to integrate this information directly into the hardware config. For instance, if a microscope.py file creates this dictionary, is somewhat obscures what devices are available. Perhaps using multiple scripts, one per file/device, could simplify this setup?

We did experiment with several options, and we found it was becoming complicated quite rapidly when different files are used. Especially if the devices must communicate with each other. The philosophy we have now is that all set-up is done in the Python script. This includes 'hidden' stuff, such as making different devices reference each other, setting parameters that we don't want to be vissible in Micro-Manager, or cannot make visible because they have non-supported data types, loading calibration files, etc., etc. The advantage of this approach is that you can use a Python program for setting up the devices in very advanced scenarios that cannot be handled purely in a configuration script. A good compromise may be to add functionality to view and edit the Python script in Micro-Manager directly.

  • If I understand correctly, one hub device corresponds to one python interpreter. Is it possible to multiple hubs/interpreters? For example, if there were an inefficiently written python device that is constantly holding the GIL, could making multiple python devices be a way around this?

Technically this is possible if the interpreters use the same runtime and virtual environment. However, is multiple hubs need different virtual environments (possibly with different Python versions) a problem occurs since only one Python runtime can be loaded once at the moment. Changing this would be very complex and error prone.

  • There is a discrepancy between the required properties and methods for Python implementations of devices compared to MM's requirements. In certain cases where MM uses methods, Python uses properties, which could lead to confusion. On the other hand, some things are easier/more natural to express as properties on the python side. So at some point it would be great to hear more about your thinking on this, and I think it would be beneficial to establish a rule for when Python devices should mirror MM's approach versus implementing a simpler alternative.
  • Consider renaming left and right properties in the camera device to contain "roi" for naming consistency with the rest of the MM APIs

Agreed, we should think about the naming. Is there a list of standard property names for the different device types that I can use?

  • Regarding python side camera API, it is quite different from the Camera MMDevice, and it currently only supports snapping image. This is something I suspect that will require a good bit out thought/discussion regarding if and how this should differ from MMDevice, as well as how it fits with the new camera API (More feedback and prototyping of new camera API (chapter 3) #243), which I plan to move towards finalizing soon.

Let's discuss this ones the new camera API is implemented. openwfs supports more advanced triggering, asynchronous acquisition and multi-threaded data processing. PyDevice only uses a subset of the openwfs camera API at the moment, but I think it can be expanded relatively easily when the new MM camera API is ready.

@henrypinkard
Copy link
Member

I think this is more than ready to merge and get to next steps at this point. @marktsuchida last things:

the Python/NumPy dependency in the build (in 3rdpartypublic; directory names will have to change and I'll update the project file).

I'm happy to take care of this--seems straightforward unless there's something I'm missing.

@IvoVellekoop Can you remind me since I've lost track--is there any preferred version of python/numpy this must build with?

There are other administrative things (for example, .gitignore shouldn't suddenly blanket-exclude .dlls at this time) that I'll also fix outside of the PyDevice directory.

I don't understand why this would be an issue, since the .girignore is in a subdirectory. But seems like something that need not block merge for now?

@marktsuchida
Copy link
Member

@henrypinkard Thanks for the nudge. I've already started to add Python to 3rdpartypublic and check the build. Should be ready today.

The .gitignore in question is not in a subdirectory.

We do not want to ignore all .dll files in the repo currently.
@marktsuchida
Copy link
Member

marktsuchida commented Jun 6, 2024

@IvoVellekoop I notice that stable.cpp is not included in the vcxproj. Is this intentional?

(It also looks like a copy of stable.cpp went to enc_temp_folder, which looks like an accident.)

@marktsuchida
Copy link
Member

Python (3.9, Windows x64) libs and include are now at 3rdpartypublic r217 Python/cp39-win_amd64 (added the version/platform so that we won't have trouble if changing/adding later). (I realize the Python headers are not currently used, but in case we need them in the future.)

Hope you don't mind I pushed 2 commits, one to update this path and another to fix the .gitignore issue.

As far as I'm concerned, the only thing blocking the merge is the stable.cpp file situation (see above). Thanks for your patience with this process.

@IvoVellekoop
Copy link
Contributor

@marktsuchida. Good to hear that PyDevice is about to be merged, thanks for the help!

I was not aware of any changes in the global .gitignore. If we made changes, they were not intentional and should indeed be reverted.

As for the path of python39.lib, that makes sense. I've modified the build instructions in README.md accordingly.

stable.cpp was listed as a Resource file rather than a Source file in vcxproj.filters. I fixed it now. There should definitely not be a copy of it in 'enc_temp_folder'. It may be something that Visual Studio did, but it was not intentional. I've removed the file from the repo.

@henrypinkard PyDevice does not rely on any specific version of Python anymore. However, for building we need python3x.lib so that the compiler knows which functions to look for in the delay-loaded dll. numpy is not needed at all anymore. The use of astropy and annotated_types is supported and completely optional.

@marktsuchida
Copy link
Member

Thanks, @IvoVellekoop! Looking forward to working together more after the merge. Thanks again for your major contribution.

@marktsuchida marktsuchida merged commit 9008c84 into micro-manager:main Jun 11, 2024
@marktsuchida
Copy link
Member

marktsuchida commented Jun 11, 2024

It looks like the CI job needed to pick up this change in the main micro-manager repo is not running (stuck on "Waiting for a runner to pick up this job..." for over 30 min).
Hopefully it's a temporary hiccup on GitHub; if it doesn't fix itself overnight I'll look into it again. If this doesn't show up in today's nightly build, this is why.

EDIT: This is now resolved.

@henrypinkard
Copy link
Member

@marktsuchida I just pulled the latest 3rd_party_public but did not see the python .lib file in there as described in instructions. Is is possible you did not commit this? (Not sure how this would be correctly building if so...). Any thoughts?

@marktsuchida
Copy link
Member

@henrypinkard It's there, otherwise the build wouldn't work.... The latest revision is 3rdpartypublic r217 (not 3rd_party_public).

$ ls Python/cp39-win_amd64/libs
_tkinter.lib python3.lib  python39.lib

@henrypinkard
Copy link
Member

Huh, weird that I didn't see it.

Does that trac website where one can view this repo still exist or is the only way to inspect to pull to local?

@marktsuchida
Copy link
Member

For now, https://valelab4.ucsf.edu/svn/3rdpartypublic/Python/cp39-win_amd64/ (certificate is currently expired).

@IvoVellekoop
Copy link
Contributor

Was this resolved now? I've cloned the repo and built from scratch this morning to confirm that only python39.lib is needed. So that file indeed needs to be included in the 3rdpartypublic\Python\cp39-win_amd64 folder of the repo if we want developers to build this code out of the box.
Just let me know if I can help.

@marktsuchida
Copy link
Member

As far as I can tell, there never was a problem: the files are already in 3rdpartypublic (r217 and later). The automated build is already using those files to produce the PyDevice DLL shipped with the nightly build.

@henrypinkard
Copy link
Member

I think this must have been a problem with my subversion client, since the file is there. We were able to build a new version of it, though there were some difficulties along the way to getting the runtime version of python to work. I'll make a new post with details shortly

@henrypinkard
Copy link
Member

Hi all, I've made a new repository (https://github.com/micro-manager/pymmdevice) to house discussions and development of this project moving forward. I look forward to more good discussions there!

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.

5 participants