Cli - Githubissues

thewtex commented 1 year ago

@bnmajor this is scratch WIP, related:

hypha
imjoy-rpc hypha support

CC @oeway

thewtex commented 1 year ago

@bnmajor could latest updates please be pushed?

bnmajor commented 1 year ago

@bnmajor could latest updates please be pushed?

@thewtex Updated!

thewtex commented 1 year ago

@bnmajor @oeway starting with hypha server infra.

@bnmajor initialization, await's, etc. needs tweaks?

Needs to be run with https://github.com/amun-ai/hypha/pull/432

oeway commented 1 year ago

@thewtex https://github.com/amun-ai/hypha/pull/432 is merged now, docs here: https://ha.amun.ai/#/getting-started?id=serve-static-files

thewtex commented 1 year ago

@oeway thank you!!

thewtex commented 1 year ago

@oeway in the latest push, I am getting the errors in the command line console:

ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store hosHnGWDuRUhAKD6djMqmo.PxPE2ynCb6vFYX9iXZBGEH')>
Exception: Failed to get session store hosHnGWDuRUhAKD6djMqmo.PxPE2ynCb6vFYX9iXZBGEH

why is this and what does it mean?

oeway commented 1 year ago

It might because of the local proxy was removed when you call a certain function. I just tried your cli branch, the following worked (upgraded imjoy-rpc==0.5.21, and hypha==0.15.15):

    ....
    await poll_for_services(server)
    print('services found')

    services = await server.list_services()
    service = [s for s in services if s["name"] == "itkwidgets_client"][0]
    itk_viewer = await server.get_service(service)
    image = itk.imread(data)
    await itk_viewer.setImage(np.array(image))

I can see the image get loaded in the opened browser tab.

I saw you have some polling functions in the cli.py, maybe a better way to implement it is to register a service in Python, e.g. a image zarr store provider service. We register it before we trigger the browser, when browser is loaded, the viewer use the workspace/token to get the zarr store from the service. Something like this:

await server.register_service({
      "name": "ItkwidgetsFileLoader",
      "id": "itkwidgets-file-loader",
      "description": "Load a file from the server",
      "config": {
          "visibility": "protected"
      },
      "load_file": load_file
})

In the itk-vtk-viewer, we can get the service and load the remote zarr store, in a passive way. Alternatively, you can also pass the current itk-vtk-viewer api object to the load_file function, then the server will be able to control the viewer. Note that since hypha service function are suppose to be functional, so you won't be able to keep the viewer api object.

Another advantage is we can have multiple viewer window open at the same time.

We can support collaborative visualisation too, since the server service can push viewer parameters to the viewer clients.

bnmajor commented 1 year ago

@thewtex @oeway Thank you both for all of your help understanding what we actually need here!

This branch has been updated to register services for communication between the client and server rather than registering a plugin and seems to work well for now! There is basic arg support for some of the args the createViewer expects and can be expanded to add support for all of the other kwargs that view currently accepts. It is still hard-coded to handle images using itk.imread and does not currently handle point_set files though. Before doing too much here though I thought it might be good to discuss best options for how to handle all the possible file inputs.

Additionally there is a convenience function now, standalone_viewer, which can be used to retrieve the itk-vtk-viewer object when a viewer has been created and served. Right now it just expects the full URL which is then parsed to use the workspace and token to connect to the server before using the service to get the viewer. The easiest way to use this is by starting IPython from the command line:

In [1]: from itkwidgets import standalone_viewer
In [2]: url = "http://127.0.0.1:37480/itkwidgets/index.html?workspace={...}&token={...}"
In [3]: viewer = await standalone_viewer(url)
In [4]: await viewer.setRotateEnabled(True)

This is achievable through the Python REPL as well but it is a bit more complicated:

>>> from itkwidgets import standalone_viewer
>>> import asyncio
>>> async def start_rotate(loop, url):
...     viewer = await standalone_viewer(url)
...     await viewer.setRotateEnabled(True)
>>> url = "http://127.0.0.1:37480/itkwidgets/index.html?workspace={...}&token={...}"
>>> loop = asyncio.new_event_loop()
>>> future = loop.create_task(start_rotate(loop, url))
>>> loop.run_until_complete(future)

oeway commented 1 year ago

FYI: i recently added “startup function” support which might be useful for this cli. Not sure how well it will fit the cli case though: https://ha.amun.ai/#/?id=custom-initialization-and-service-integration-with-hypha-server

oeway commented 1 year ago

Hi @bnmajor I added a synchronous wrapper for imjoy-rpc with hypha server: https://github.com/imjoy-team/imjoy-rpc/pull/546 (It works by running the event loop in a separate thread) This might improve how the user use the itkwidgets cli since they won't need to use async/await. Let me know if it works for you.

bnmajor commented 1 year ago

I added a synchronous wrapper for imjoy-rpc with hypha server: imjoy-team/imjoy-rpc#546 (It works by running the event loop in a separate thread)

Thanks @oeway, this is fantastic! With this change I was able to update the function to be synchronous, which means that starting up a basic python interpreter and updating the viewer programmatically is now significantly simpler:

>>> from itkwidgets import standalone_viewer
>>> url = "http://127.0.0.1:37480/itkwidgets/index.html?workspace={...}&token={...}"
>>> viewer = standalone_viewer(url)
>>> viewer.set_rotate_enabled(True)

This seems to work well for basic functionality, but if you try to set an image or label image you get the following:

ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.ZSuEC7FJWDivJZCDgdvLst')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.ZSuEC7FJWDivJZCDgdvLst
ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.nvfDB8jVRH9oGHLv3jaFx6')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.nvfDB8jVRH9oGHLv3jaFx6
ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.hRE4ARjrx6i4RFhnkDQxLM')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.hRE4ARjrx6i4RFhnkDQxLM
ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.9hpyjmbg3gjWYwaDvqY6j4')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.9hpyjmbg3gjWYwaDvqY6j4
ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.C5mTujPBemgJHiMhDf3AvR')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.C5mTujPBemgJHiMhDf3AvR
ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.J8NVuLMh9dYdbUpyoCNeSj')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.J8NVuLMh9dYdbUpyoCNeSj
ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.EEfogoASEdFZPjk3EkJfey')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.EEfogoASEdFZPjk3EkJfey
ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.hGJQprwuK6k3g63VnM8ox7')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.hGJQprwuK6k3g63VnM8ox7
ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.QGZbnUDgounxW965MoVHpj')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.QGZbnUDgounxW965MoVHpj
ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.DapoRmvqGXH83zJpQUXrtx')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.DapoRmvqGXH83zJpQUXrtx
ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.RRwihdq3r93Nx26G7zocEG')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.RRwihdq3r93Nx26G7zocEG
ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.jpY6hpDbs2aFY9bBYQxrzP')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.jpY6hpDbs2aFY9bBYQxrzP
ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.Ui5mEzzmcuNeNJ2HDTMcgu')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.Ui5mEzzmcuNeNJ2HDTMcgu
ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.CNaMTuV4kgUxA76dvzDzGX')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.CNaMTuV4kgUxA76dvzDzGX
ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.A9JK8qYBja5AT32tTypiyw')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.A9JK8qYBja5AT32tTypiyw
ERROR:asyncio:FuturePromise exception was never retrieved
future: <FuturePromise finished exception=Exception('Failed to get session store QKVLVpfxrfj3KzeyayFKLS.5ja3V8Xmpyditskz35JPSM')>
Exception: Failed to get session store QKVLVpfxrfj3KzeyayFKLS.5ja3V8Xmpyditskz35JPSM

@thewtex The function has also been updated to return a Viewer instance so that all functions are wrapped the same way that they are in the notebook.

bnmajor commented 1 year ago

FYI: i recently added “startup function” support which might be useful for this cli. Not sure how well it will fit the cli case though: https://ha.amun.ai/#/?id=custom-initialization-and-service-integration-with-hypha-server

@oeway I did look into using this approach, but it seems that the server object that is returned has no register_codec function - is this intentional?

oeway commented 1 year ago

@oeway I did look into using this approach, but it seems that the server object that is returned has no register_codec function - is this intentional?

Have you tried the latest version, it should work:

>>> from imjoy_rpc.hypha import connect_to_server_sync
>>> server = connect_to_server_sync({"server_url": "https://ai.imjoy.io"})
>>> server.register_codec
<bound method RPC.register_codec of <imjoy_rpc.hypha.rpc.RPC object at 0x7fc248082520>>

oeway commented 1 year ago

BTW, I just added webrtc support for imjoy-rpc: https://github.com/imjoy-team/imjoy-rpc/pull/547 This might be interesting for you too, it enables peer-to-peer connection between hypha clients, you can use our public server for signaling, the the rest will be p2p. We are setting up a STUN/TURN server for complicated firewall too. The plan is to support streaming microscope images and control it with hypha. It will be interesting for itkwidgets to enable remote rendering too!

bnmajor commented 1 year ago

@oeway I did look into using this approach, but it seems that the server object that is returned has no register_codec function - is this intentional?

Have you tried the latest version, it should work:
>>> from imjoy_rpc.hypha import connect_to_server_sync
>>> server = connect_to_server_sync({"server_url": "https://ai.imjoy.io"})
>>> server.register_codec
<bound method RPC.register_codec of <imjoy_rpc.hypha.rpc.RPC object at 0x7fc248082520>>

Thanks @oeway! That is great to know. What I am actually referring to is the server object that is passed into the startup function:

with subprocess.Popen([
    sys.executable, "-m", "hypha.server",
    f"--host={SERVER_HOST}", f"--port={PORT}",
    "--static-mounts", f"/itkwidgets:{viewer_mount_dir}",
    "--startup-functions=itkwidgets.standalone_server:start_viewer"
], env=hypha_server_env)

...

async def start_viewer(server):
    register_itkwasm_imjoy_codecs_cli(server)

...

File "/home/local/KHQ/brianna.major/itkwidgets/itkwidgets/imjoy.py", line 53, in register_itkwasm_imjoy_codecs_cli
    server.register_codec({'name': 'itkwasm-image', 'type': itkwasm.Image, 'encoder': encode_itkwasm_image})
TypeError: 'NoneType' object is not callable

oeway commented 1 year ago

What I am actually referring to is the server object that is passed into the startup function

@bnmajor Oh, I see, sorry for the confusion. I have fixed it now in hypha>=0.15.27: https://github.com/amun-ai/hypha/pull/472

bnmajor commented 1 year ago

@bnmajor Oh, I see, sorry for the confusion. I have fixed it now in hypha>=0.15.27: amun-ai/hypha#472

Thanks! Unfortunately I'm still seeing the same issue... I'm using hypha 0.15.27 and imjoy-rpc 0.5.31.

oeway commented 1 year ago

@bnmajor Oh, I see, sorry for the confusion. I have fixed it now in hypha>=0.15.27: amun-ai/hypha#472

Thanks! Unfortunately I'm still seeing the same issue... I'm using hypha 0.15.27 and imjoy-rpc 0.5.31.

Hi, sorry, missed another place which requires change, now added as a test case. It should work with hypha>=0.15.28 (available soon).

bnmajor commented 1 year ago

Hi, sorry, missed another place which requires change, now added as a test case. It should work with hypha>=0.15.28 (available soon).

Thanks for all of the updates and fixes! Unfortunately in testing out the functionality a bit more I think that this may not be the move for our use case. We are using the services to trigger the next steps in the pipeline server-side, and this requires sharing some global state variables. Using the the startup function creates a new instance of the module so this global state is not available. If I am maybe missing something here please let me know though!

bnmajor commented 1 year ago

@thewtex This PR currently:

Is started with the cli entry point: itkwidgets --image {...} {...optional args}
Can accept a --verbose flag to output stdout to the terminal again, otherwise output is surpressed
Can accept a --repl flag which will automatically start an interactive REPL session
- This session makes the viewer object available and this is explained in the banner created with the session
The issue mentioned above regarding setting the label image seems to be the same issue described [before] (https://github.com/InsightSoftwareConsortium/itkwidgets/pull/589#issuecomment-1586218575). This issue happens at the same place (in updateRenderedImage) and only for setImage and setLabelImage where we queue the functions. Creating and calling a simple asynchronous function succeeds.

oeway commented 1 year ago

Thanks for all of the updates and fixes! Unfortunately in testing out the functionality a bit more I think that this may not be the move for our use case. We are using the services to trigger the next steps in the pipeline server-side, and this requires sharing some global state variables. Using the the startup function creates a new instance of the module so this global state is not available. If I am maybe missing something here please let me know though!

I see, then I think you are right, startup functions won't work in this case.

bnmajor commented 1 year ago

@thewtex The bug that we discussed where we cannot load a label image still exists but it is not obvious why... Thanks to @PaulHax we know that the exact issue is that getItem is failing for some reason (here). What I know so far:

If we pass the image and label data to the client via the inputObject service function and then we pass in all of the data without the label image we can call await viewer.setLabelImage right after viewer resolves and it works.
If we try to call viewer.set_label_image (the wrapper for the client-side viewer.setLabelImage function) from the Python REPL session it fails at getItem.
If we create a client-side service function that calls viewer.setLabelImage and pass it the image after it is loaded from the Python REPL session it fails at getItem.
All approaches are taking the same path on the server-side to pass the same multiscale zarr store to the client.
The only other difference I can see between the successful and failed approaches is that it appears that any time the client is in charge of requesting the data we have no issues, but any time that the server sends the data without a client request it fails. This makes me think maybe there is a codec issue? But if so shouldn't we see the same issue in the notebooks where we register the same codecs and pass the data from the server in the same manner?

@oeway I am leaning towards this being something that I am doing wrong (or fundamentally misunderstanding) rather than an issue in the itk-vtk-viewer code, so if you have any thoughts or ideas I would welcome them!

thewtex commented 1 year ago

@bnmajor great work debugging.

Another debugging session with @PaulHax , a few more interesting points:

We confirmed that getItem is not returning on the JavaScript side, but only with the array chunks -- it succeeds for .zattrs and .zarray.
We tried a few more approaches, and we confirmed your observation that it succeeds only when pulling from the JavaScript side. The call flow: set_label_image passes a remote zarr store on which the JS side will make remote procedure calls before completing do not succeed.

It appears that there is some issue with our syncified Python RPC that requires JS RPC calls before returning?

In any case, is there a workaround that we can use to behave like the initial input data objects, pulling client-side?

oeway commented 1 year ago

@bnmajor great work debugging.

Another debugging session with @PaulHax , a few more interesting points:

We confirmed that getItem is not returning on the JavaScript size, but only with the array chunks -- it succeeds for .zattrs and .zarray.

We tried a few more approaches, and we confirmed your observation that it succeeds only when pulling from the JavaScript side. The call flow: set_label_image passes a remote zarr store on which the JS side will make remote procedure calls before completing do not succeed.

It appears that there is some issue with our syncified Python RPC that requires JS RPC calls before returning?

In any case, is there a workaround that we can use to behave like the initial input data objects, pulling client-side?

I think what you are observing is related to a dead lock in the syncified python RPC. It happens because we execute the syncified functions in the main thread. If we have a service function A registered, normally, it will respond to remote calls from the js side. However, if we initiate a call from the same main thread to call something on the js side, then the js side try to call A again, the thread gets locked because it will occupy the thread and function A will never get executed.

I need to check if I can solve it inside the rpc library. But for now, an easy way to solve this is to create two server connections and use one to register the internal services, and the other one for accessing from the Python REPL.

Edit: Fixed now, I increased the worker number in the thread executor of the syncifier and it seems fixes the issue of deadlock. Please upgrade imjoy-rpc to 0.5.42 and let me know if it works.

Thanks for reporting!

@thewtex @bnmajor

thewtex commented 1 year ago

I added a commit that adds terminal output by default! :computer: :nerd_face: We can still open in a browser with the -b or --browser flag, but this enables quick terminal-based inspection of 3D data. It requires a terminal that supports the iterm2 inline image protocol such as:

wezterm
VSCode's Terminal (with the latest version of VSCode)
iTerm2

In VSCode:

itkwidgets-vscode.webm

thewtex commented 1 year ago

VSCode terminal + tmux. Requires set-option -g allow-passthrough on in ~/.tmux.conf.

itkwidgets-vscode-tmux.webm

thewtex commented 1 year ago

wezterm on Linux:

itkwidgets-wezterm-linux.webm

thewtex commented 1 year ago

Verified on Windows 10, wezterm, ssh'ing into an ARM mac:

and Windows ssh'ing into a headless Linux:

wezterm with Windows 11 WSL with itkwidgets running on Windows also works 🎉 🌮

Install command will be:

pip install 'itkwidgets[cli]'
playwright install --with-deps chromium

bnmajor commented 1 year ago

@thewtex We can now set images and label images! :slightly_smiling_face:

Edit: Fixed now, I increased the worker number in the thread executor of the syncifier and it seems fixes the issue of deadlock. Please upgrade imjoy-rpc to 0.5.42 and let me know if it works.

@oeway Thank you for this! I apologize, I have been out for a few days. I did have a chance to try out your changes but unfortunately I was still seeing the same issue. I did end up with a workaround where I am saving the zarr store server-side and then using services to trigger the client-side to pull the store and set the image/label. I found that this exact same approach would fail if I used a service to save the store server-side though (and any attempts to push the store rather than pull it always faill) so I am still not sure that I understand why my current solution actually works. If you have any ideas I would love to understand this better but for now at least we are able to use the full API from the REPL session! Thank you!!

InsightSoftwareConsortium / itkwidgets

Cli #589