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

RWR Implementation #12

Open
wants to merge 52 commits into
base: master
Choose a base branch
from
Open

RWR Implementation #12

wants to merge 52 commits into from

Conversation

blessyantony9
Copy link
Collaborator

Added RWR implementation using PageRank from PathLinker.

Blessy Antony added 30 commits March 31, 2021 15:50
 src/PathLinker

subrepo:
  subdir:   "src/PathLinker"
  merged:   "d4a44c9"
upstream:
  origin:   "https://github.com/Murali-group/PathLinker.git"
  branch:   "master"
  commit:   "d4a44c9"
git-subrepo:
  version:  "0.4.3"
  origin:   "???"
  commit:   "???"
@blessyantony9 blessyantony9 linked an issue Jun 22, 2021 that may be closed by this pull request
@jlaw9
Copy link
Contributor

jlaw9 commented Jun 22, 2021

Thanks @blessyantony9! I see you copied the PageRank.py to src/FastSinkSource/src/algorithms, and also added the PathLinker repo to src. It looks like the PathLinker repo isn't used anywhere, is that right? Can we remove it?

@blessyantony9
Copy link
Collaborator Author

Thank you @jlaw9 for reviewing. Yes, the aim is to reuse PageRank from within PathLinker which is added as a sub-repo in this repo. However, unfortunately, there is version incompatibility for one of the packages between PathLinker and SARS-CoV-2-network-analysis: networkX. In PathLinker it is v1.11 whereas in the current repo it is v2+. So, until we upgrade the version of this package in the whole of PathLinker, I have copied the code of PageRank over to src/FastSinkSource/src/algorithms and updated the usages only in this file as a tactical solution. I have included this as a TODO comment in the file.
I hope to update the PanthLinker repo soon and get rid of this redundant file. Hence, I have retained the sub-repo but can definitely remove it and wait until the update to networkX v2+ is complete before pulling it in.

@jlaw9
Copy link
Contributor

jlaw9 commented Jun 22, 2021

Ah ok I see. I appreciate the work you did to incorporate the networkx graphs to be able to use that PageRank implementation.

Since we already have the other algorithms implemented in terms of matrix-vector multiplication, I think it would keep the code simpler (and faster than networkx) to do the same for RWR i.e., s^(i+1) = aPs^(i) + (1-a)y. And then we don't have to worry about the networkx versions.

I had forgotten that I had added such an implementation to the annotation_prediction repo, which is the updated version of the FastSinkSource repo (I had planned to update the FastSinkSource part of this repo to use that annotation_prediction repo, but hadn't gotten around to it unfortunately).

It looks like there are a couple of features that PageRank.py code has that my rwr.py doesn't have, such as the ability to use non-uniform weights for the restart probabilities, but I think that would be easy enough to add if we we wanted to use that.

@blessyantony9
Copy link
Collaborator Author

blessyantony9 commented Jun 23, 2021

Thank you @jlaw9 for the inputs about using matrix multiplication. I was unaware of the existing matrix implementation. I too implemented RWR using sparse matrices for another project within the N3C Enclave (Palantir) which too assumes default weight=1, but as you mentioned can be tweaked to incorporate non-uniform weights. I'll discuss the available matrix implementations with Dr.Murali and update this pull request accordingly (if we decide to go with the matrix version).

@tmmurali
Copy link
Member

@blessyantony9 please go ahead and use the matrix implementation from the other project here as well.

@blessyantony9
Copy link
Collaborator Author

Thank you @tmmurali. I'll update the code to use RWR from the annotation_prediction repo.

@blessyantony9
Copy link
Collaborator Author

@jlaw9 As discussed, I've added the PageRank Matrix implementation. Apologies for the delay. Please let me know if you have any questions or if any changes are required.

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.

Add RWR implementation to pipeline
3 participants