Catalyst.jl icon indicating copy to clipboard operation
Catalyst.jl copied to clipboard

Complex balancing

Open vyudu opened this issue 1 year ago • 2 comments

Checklist

  • [x] Appropriate tests were added
  • [x] Any code changes were done in a way that does not break public API
  • [x] All documentation related to code changes were updated
  • [x] The new code follows the contributor guidelines, in particular the SciML Style Guide and COLPRAC.
  • [x] Any new documentation only uses public API

Additional context

A first pass at the complex-balance implementation. Does not seem like Graphs.jl has a matrix-tree implementation, so I have the matrix tree implementation here for now, but I'll work on adding it as a PR to Graphs.jl (and in particular implementing a method of generating the spanning trees that doesn't simply generate every combination of n-1 edges).

vyudu avatar May 12 '24 04:05 vyudu

@vyudu can you merge master into this? Do you know how to do that? (If not I can explain to you on Slack.)

I usually use Gitkraken to do that as I find it easier than merging via the commandline.

isaacsas avatar May 15 '24 12:05 isaacsas

I second that GitKraken is a really nice tool, if you haven't checked it out yet.

TorkelE avatar May 15 '24 12:05 TorkelE

CI is still failing?

isaacsas avatar Jun 04 '24 14:06 isaacsas

This looks like it might have been rebased on an older version of master?

isaacsas avatar Jun 04 '24 14:06 isaacsas

Maybe try merging master into your PR?

isaacsas avatar Jun 04 '24 14:06 isaacsas

(Generally I find it easier to merge instead of rebase to update a PR.)

isaacsas avatar Jun 04 '24 14:06 isaacsas

@vyudu I tried to merge this onto master for you, but couldn't get it to work unfortunately. If you check the "Files changed" tab at the top of the PR you see that there are a very large number of changes: image

What probably have happened is that this was a PR of some branch, and then it got rebased on an updated master, and somewhere some of the changes of the branches it originally was branched off of was carried through when this one was rebased on the master again.

Since you are developing on your own master branch, I would try (as Sam pointed out here, and to me earlier) to simply merge your master branch with Catalyst's master branch, and then push the PR again. If that does not work out, and the changes introduced here are very straightforward (i.e. just adds functions at the bottom of the network_analysis.jl file, and some tests) it might be easiest to simply copy these into a new, clean, branch and make a new PR from that.

TorkelE avatar Jun 04 '24 15:06 TorkelE

Thanks @vyudu great first PR.

isaacsas avatar Jun 06 '24 15:06 isaacsas

Thanks @vyudu great first PR.

Second that! And sorry for giving you poor advice on the rebase/merge thing. I tried @isaacsas's method properly now as well and it works great

TorkelE avatar Jun 06 '24 15:06 TorkelE

Thank you both!

vyudu avatar Jun 06 '24 16:06 vyudu