RWR and TieDIE Integration #92

Lyce24 · 2023-06-16T06:38:18Z

Summary:

This pull request introduces Docker images for RWR (Random Walk with Restart) and TieDIE algorithms, along with the necessary code implementation and tests. Additionally, it includes a new utility function and updates the GitHub Actions workflow.

Changes Made:

Created Docker images for RWR and TieDIE in docker-wrapper/RandomWalk/Dockerfile and docker-wrapper/TieDIE/Dockerfile respectively.
Implemented the algorithm execution code in src/random-walk.py and src/tiedie.py.
Added tests for RWR and TieDIE in the test/RandomWalk and test/TieDIE directories.
Updated the GitHub Actions workflow to include the new tests in .github/workflows/test-spras.yml.
Added the add_rank_column function to src/utils.py for adding an extra column to a dataframe.

Next Steps:

Transfer the Docker images from the personal DockerHub account to the official DockerHub account for the project once access is granted.

…path_file now)

…tart repo; with the modified output file, implemented the parsing function; met one error about Snakemake checking pathway.txt; need to write the tests after

…ate the README; more things to consider: how to select edges for pathway file.

… fixing bugs about running pagerank in TieDIE)

…an utility function in src/utils.py (add rank column to a dataframe); updating README for RWR and TieDIE

agitter

I didn't review everything and am only leaving a few comments about things I noticed during an initial quick pass.

agitter · 2023-06-22T01:46:06Z

.github/workflows/test-spras.yml

+        docker pull erikliu24/rwwr:latest
+        docker pull erikliu24/tiedie:latest


Adding a reminder to change these to reedcompbio after @annaritz pushes the containers to the organization account. Same goes for the steps below.

Yes, I will make changes to this file when Anna pushes the image to reedcompbio. And the changes will be made also in src/random_walk.py and src/tiedie.py as the run functions in these two files are also pulling images from my personal account.

I built and pushed both of these to DockerHub so you can make the test workflow changes now:

https://hub.docker.com/repository/docker/reedcompbio/random-walk-with-restart

https://hub.docker.com/repository/docker/reedcompbio/tiedie

config/config.yaml

docker-wrappers/TieDIE/Tutorial.pdf

src/util.py

test/TieDIE/test_tiedie.py

config/config.yaml

docker-wrappers/TieDIE/README.md

ntalluri · 2023-06-30T23:10:25Z

src/rwr.py

+        Access fields from the dataset and write the required input files
+        @param data: dataset
+        @param filename_map: a dict mapping file types in the required_inputs to the filename for that type
+        @return:


empty return comment

I left this empty because all the other algorithms implemented in SPRAS also have an empty return comment. I would appreciate your recommendation on what should be included here. If you have any suggestions, I can update the return comment for this algorithm as well as for the other algorithms in SPRAS.

Because generate_inputs doesn't return anything I suggest deleting the @return: line from the docstring. We'll have to formalize this once we finally start automatically generating documentation from docstrings with something Sphinx.

ntalluri · 2023-06-30T23:22:44Z

src/rwr.py

+        @param raw_pathway_file: pathway file produced by an algorithm's run function
+        @param standardized_pathway_file: the same pathway written in the universal format
+        """
+        print('Parsing random-walk-with-restart output')


is this needed?

I had initially considered printing this line to the console to provide users with additional information about the progress of the program. However, I can remove this line if it would be more in line with the unified coding style of SPRAS.

We can remove this parsing output message. We do some logging when it is helpful for debugging, like the Python commands that are going to be run inside the Docker containers. Otherwise for general information about workflow progress we rely on the Snakemake output.

src/rwr.py

src/tiedie.py

ntalluri · 2023-06-30T23:45:37Z

I will do a second pass through, but this is what I caught fast through my first pass through

…ytest -k; Tab issue of RWR); updated parameters for RWR for newer version of RWR

agitter

I build and pushed the Docker images so that you can keep making progress on this. I still haven't looked at the core Python code.

docker-wrappers/RWR/README.md

docker-wrappers/RWR/Dockerfile

docker-wrappers/RWR/README.md

agitter · 2023-07-08T02:34:37Z

src/rwr.py

+        @param raw_pathway_file: pathway file produced by an algorithm's run function
+        @param standardized_pathway_file: the same pathway written in the universal format
+        """
+        print('Parsing random-walk-with-restart output')


We can remove this parsing output message. We do some logging when it is helpful for debugging, like the Python commands that are going to be run inside the Docker containers. Otherwise for general information about workflow progress we rely on the Snakemake output.

test/RWR/test_rwr.py

test/TieDIE/test_tiedie.py

… Images and made suitable changes based on this)

… RWR_and_TieDIE

…R, and updated the config.yaml to add more tests

annaritz · 2023-07-24T23:03:57Z

We're reviewing this now, specifically the RWR code. It currently outputs a single file with both edge and node information (for example, the node visitation probabilities). This reminded us of #88, or is there a way to keep intermediate files besides a raw-pathway.txt file? It would help to have this information about the nodes, but at the same time we don't want to introduce a change that requires adjusting ALL algorithms (unless we really vet that change).

agitter · 2023-07-25T15:08:45Z

If I understand correctly, it is awkward to stick the node information in raw-pathway.txt because that file is edge-based but the node scores are node-based. Snakemake should allow us to write additional output files per rule as long as the rule writes the expected output file(s). Would it work if we use the output_file argument to the run function to get the output directory and create a node-based file of your choice to write a node score file? It would be a one-off output file that isn't currently used by any downstream steps, but it would be there for each RWR run and available for inspection or manual visualization.

Because writing this additional output file is atypical SPRAS behavior, I suggest clearly documenting it in the run function docstring and perhaps the RWR docker wrapper readme as well.

ntalluri · 2023-08-16T15:29:01Z

for the testing of tiedie and RWR, can we add a test to check for bidirectional edges in the input pathway/edges?

ntalluri · 2023-08-22T18:10:32Z

src/util.py

+    Add a column of 1s to the dataframe
+    @param df: the dataframe to add the rank column of 1s to
+    """
+    df['rank'] = 1


if we are going to show the headers in the future (because we have a bunch of files that look the same but have different meanings), most of the other algorithms have 'Rank' for the column name for the rank column added. It's a minor thing, but I think this can be changed to also use 'Rank' for uniformity. Also since this is being added, we should update the rest of the algorithms that are manually adding a rank column of 1s to use this function.

In #120 I updated the other algorithms to use the util function to add the rank column because you already modified them to add the directionality column to the standardized pathway output

agitter · 2023-12-03T21:39:39Z

#120 adding directionality has been merged, so we can work on incorporating those changes here. It also introduced some merge conflicts we'll have to resolve.

Lyce24 · 2024-02-11T03:23:13Z

I'm closing this pull request and creating separate pull requests for RWR and TieDIE with the directionality included.

agitter · 2024-02-11T13:47:16Z

Sounds good @Lyce24. When you create the new pull requests, please include a comment like Replaces #92 so we can refer back to the old comments here if needed.

erikliu and others added 14 commits June 9, 2023 00:14

Create Docker Image for random-walk

ecec5b6

Implementing src/random_walk.py (questions about the output files)

34d3ef3

Updating Dockerfile and src/random_walk.py (new image and single raw_…

aff0893

…path_file now)

Make RWWR working (change Dockerfile for current random-walk-with-res…

5755c62

…tart repo; with the modified output file, implemented the parsing function; met one error about Snakemake checking pathway.txt; need to write the tests after

Make the program running (writing the tests); clean up the codes; upd…

13dd12a

…ate the README; more things to consider: how to select edges for pathway file.

Initiate dockerizing TieDIE repo

2e6aaa5

Dockerize TieDIE and integrate it into SPRAS

c214002

Fixing the format of TieDIE output pathway; need to consider RWwR

78036de

Update RandomWalk and TieDIE (add user-defined threshold for RWwR and…

45d5733

… fixing bugs about running pagerank in TieDIE)

Complete the tests for RWR and TieDIE (test and GitHub actions); add …

fafbff9

…an utility function in src/utils.py (add rank column to a dataframe); updating README for RWR and TieDIE

Merge branch 'master' into RWR_and_TieDIE

c958aa3

Fixing pre-commit hooks and updating the GitHub Action

fe863e0

Fixing the GitHub Action

62fc246

Updating the Docker Image for RWR and TieDIE

becf7bc

agitter requested changes Jun 22, 2023

View reviewed changes

Erik_L added 2 commits June 22, 2023 14:19

Completing changes requested

b7d2c94

Renaming random_walk to rwr

8e94cf3

Lyce24 force-pushed the RWR_and_TieDIE branch from a2c94e4 to 8e94cf3 Compare June 29, 2023 00:00

Fix a minor bug in workflows/test-spras.yaml (RandomWalk to RWR)

f467db1

ntalluri requested changes Jun 30, 2023

View reviewed changes

Resolved some issues suggested by Neha (updating configfile; adding p…

7833ea5

…ytest -k; Tab issue of RWR); updated parameters for RWR for newer version of RWR

agitter reviewed Jul 8, 2023

View reviewed changes

Lyce24 and others added 7 commits July 9, 2023 02:27

Merge branch 'Reed-CompBio:master' into RWR_and_TieDIE

f7f2ba4

Resolved some issues suggested by Tony (pushed the ReedCompBio Docker…

53faeb0

… Images and made suitable changes based on this)

Merge branch 'RWR_and_TieDIE' of https://github.com/Lyce24/spras into…

b7f007f

… RWR_and_TieDIE

Pre-commit check

269c14e

Merge branch 'Reed-CompBio:master' into RWR_and_TieDIE

dd75af8

Added single source mode to RWR, updated the input file format for RW…

1f5a233

…R, and updated the config.yaml to add more tests

Updated tests for RWR

b2d6a8e

Minor updates for RWR (param updates; test updates)

4e25538

agitter mentioned this pull request Aug 18, 2023

Update contribution guide #97

Merged

ntalluri reviewed Aug 22, 2023

View reviewed changes

Lyce24 closed this Feb 11, 2024

Lyce24 mentioned this pull request Feb 21, 2024

Add RWR pathway reconstruction algorithm #148

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RWR and TieDIE Integration #92

RWR and TieDIE Integration #92

Lyce24 commented Jun 16, 2023

agitter left a comment

agitter Jun 22, 2023

Lyce24 Jun 22, 2023

agitter Jul 8, 2023

ntalluri Jun 30, 2023

Lyce24 Jul 4, 2023

agitter Jul 8, 2023

ntalluri Jun 30, 2023

Lyce24 Jul 4, 2023

agitter Jul 8, 2023

ntalluri commented Jun 30, 2023

agitter left a comment

agitter Jul 8, 2023

annaritz commented Jul 24, 2023

agitter commented Jul 25, 2023

ntalluri commented Aug 16, 2023 •

edited

Loading

ntalluri Aug 22, 2023

agitter Dec 3, 2023

agitter commented Dec 3, 2023

Lyce24 commented Feb 11, 2024

agitter commented Feb 11, 2024

		docker pull erikliu24/rwwr:latest
		docker pull erikliu24/tiedie:latest

RWR and TieDIE Integration #92

RWR and TieDIE Integration #92

Conversation

Lyce24 commented Jun 16, 2023

Summary:

Changes Made:

Next Steps:

agitter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ntalluri commented Jun 30, 2023

agitter left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annaritz commented Jul 24, 2023

agitter commented Jul 25, 2023

ntalluri commented Aug 16, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agitter commented Dec 3, 2023

Lyce24 commented Feb 11, 2024

agitter commented Feb 11, 2024

ntalluri commented Aug 16, 2023 •

edited

Loading