Closed jim-rafferty closed 4 years ago
I think I might have messed this up, because it's saying "wants to merge 4 commits into pnnl:master from jim-rafferty:weighted_hypernetworks"
Can create a new branch and change the PR to point to that or do I need to do something else?
Sorry to make you the guinea pig in all of this. I created a branch for you to push to: feature/weighted_hypernetworks Please push to that branch. I need to figure out the correct work flow for outside contributors. We don't push anything directly to master. In house we push to develop and make a pull request from develop to master. That isn't probably the best way for outside collaborators as we could have clashes. So ideally an outside collaborator would push to a feature or hotfix branch and we would review before merging. I will have to look into that. If you find a good source describing the correct workflow, let me know. Thanks @brendapraggastis
Done. Thanks :)
@jim-rafferty Your changes are scheduled to go into our next release, hopefully early September. Thanks!
Hi @brendapraggastis Thank you very much! I really appreciate your time and effort. :)
Hi @jim-rafferty, I am afraid our current code is too far ahead of your merge request to merge without conflicts. Our current code is tagged v0.3.8 If you would still like to merge your changes could you check out the tagged version and send me an update. I can make the changes myself if you would prefer. Sorry for the inconvenience. Brenda
Here is a new PR for the weighted hypernetworks code, with all the new commits in a separate branch. Here is the original comment for reference:
I thought I would try to make it a bit easier to create a weighted hypergraph, using a similar syntax to that in networkx. Now you can add weighted edges like this:
I've also added support for calculating the weighted
nodeadjacency matrices:I have an issue with the edge adjacency matrix, because I would ordinarily calculate it with the following matrix formula: \sqrt(W) M^T M * \sqrt(W), but this won't currently work asedge_adjacency_matrix
calls__incidence_to_adjacency
, the same method as is called byadjacency_matrix
. I would be grateful for your thoughts on this, I've currently left the weighted edge adjacency to throw aNotImplementedError
.I have modified the code to do what I commented in the original PR. When the weighted adjacency matrix is requested,
M.dot(weight_matrix)
is passed into__incidence_to_adjacency
so both adjacency matrix functions work.All of these changes play quite nicely with the existing code, but I wanted to submit them for your consideration before doing too much more.
Many thanks. :)