-
Notifications
You must be signed in to change notification settings - Fork 20
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
Cytoscape #66
Conversation
Thanks for working on this. Cytoscape visualization will be a great addition. I'll work on input for the Snakefile rule to gather all of the pathway files. The design here can be streamlined compared to what we set up for TPS because Cytoscape will always run inside the container. All we need on the SPRAS side is an interface to pass the list of files (and any annotations) to the container. We should also try to avoid conda inside the container and install the required packages with pip. That will give us a chance of creating a container that works with Singularity. Several files in this pull request will be deleted later so we should squash and merge when it's time. |
We need to decide which pathways should be visualized in the Cytoscape session. Should it be the pathways that correspond to the parameters in the current config file (current behavior). Or all pathways discovered in the output directory, including some that may be left over from previous runs with parameters that were removed from the config file? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm able to run the workflow now locally with Docker and generate the Cytoscape session file. It's very cool to have it working. I did get what looks like several error messages from the cyrest client that I haven't looked at closely.
All of the Docker-related files can be moved to docker-wrappers/Cytoscape
from src/analysis/py4cy
. Before we merge, please add a readme describing this image. We'll also want to switch to the reedcompbio DockerHub organization.
Can we delete the two empty __init__.py
files?
The workflow runs to completion when the Cytoscape step is skipped, which is good because earlier it looked like it might be failing at one of the PathLinker runs. I now suspect the issue is #22 and that the output .cys file is owned by root. Switching to the |
Remove CMD and track py4c version Remove some Python logging statements Extend argument parsing
No longer reports correct pathway filenames
I pushed some updates to these Cytoscape files today. Some were minor code cleanup and reorganization changes. I converted the container wrapper to use the I was able to simplify the Docker call somewhat, which makes me more optimistic about Singularity support. The There are a few other open TODOs:
|
I pushed reedcompbio/py4cytoscape to DockerHub so we can test and maintain that version. Update: the tests now pass with this version of the Docker image and the |
I was able to get the workflow to run with the new docker image and the changes to the cytoscape backend components. |
I'm now manually creating the Cytoscape vmoptions file and adding it to the image. I ran the vmoptions generation script inside an interactive container session and modified the memory settings and set the home directory to point to the writeable This post states that the |
I posted to the Cytoscape Google group about the Singularity problems I'm having and received a response from one of the developers: https://groups.google.com/g/cytoscape-helpdesk/c/NNC1Bj8fcPA/m/4HOlPmvtAQAJ That gives me some hope for continuing to debug this. I'll do more interactive Singularity testing based on the feedback. |
Cytoscape ignores -Duser.home in vmoptions for some cache files
Through interactive testing in Docker and Singularity and support from a developer through the Google group, I figured out the Singularity "No space left on device" error. I also believe I have a workaround. The error was due to Cytoscape not using the I was eventually able to get Singularity to recognize a different home directory. Now Cytoscape runs interactively with Singularity in my converted Docker image. I also fixed a Cytoscape bug, which I believe is new, described here using their recommended workaround. Despite that, the workflow still hung at the Cytoscape rule in Singularity. There are still some final changes to make after the workflow runs:
|
This works in Singularity now! I also refactored it to produce one Cytoscape session per dataset instead of combining all pathways into a single dataset. I write the temporary Cytoscape logs to a dataset-specific output directory and then remove it after Cytoscape finishes, so there shouldn't be any problems with parallelism. I'd like to look over all the code once more and then will ask someone else to test that it works before merging. |
I added test cases to run Cytoscape in Docker and Singularity. If those pass, I'll tag the image as @ntalluri could you also please try testing the workflow on your machine in Docker to confirm you get session files and no errors before I merge? |
The
My test server has Apptainer 1.2.2 whereas GitHub Actions is using 1.1.3.
The 1.2.0 release introduced some important behavioral changes so I'll update what we run in GitHub Actions to see if that is causing the test failure. |
All tests pass (after marking the Cytoscape Singularity test xfail) and I tagged the image as v1. @ajshedivy I'm merging in the SPRAS Cytoscape pull request! |
Let's focus on creating a cytoscape rule that can grab all of the generated pathways. After that I can continue work on creating the cytoscape session using docker. Right now, local support of cytoscape is integrated, make sure to start cytoscape, then run the workflow.
Links