search

CASCI-lab / CANA

CANAlization: Control & Redundancy in Boolean Networks
https://casci-lab.github.io/CANA/
MIT License
22 stars 15 forks source link

fix Derrida coefficient normalization #15

Closed xuan-w closed 4 years ago

xuan-w commented 4 years ago

Github is getting weird! Since the previous PR was merged, there should be only on commit waiting for merge. But it showed two. Anyway, there are only two lines of difference.

rionbr commented 4 years ago

@xuan-w , I don't think you should change the output format. If I recall, we changed from int to str because it's faster. Can you adapt the derrida curve to fit the new format? Have you ran the nosetest to make sure this doesn't impact the rest of the package? Cheers!

xuan-w commented 4 years ago

Hi Rion,

Let me clarify two things.

  1. This PR doesn't contain anything involving output format. It only contains patch to Derrida co-efficient per Alex's request. That function returns a float.

Somehow github has problem to show the commits need to merge. It wrongly declared that there are 5 commits to be merged, actually there are only two. I merged all changed from master branch before sending the PR. If you look at "Filed changed" tab, you will see that actually there are only three lines change, they are on the DC function.

  1. My previous PR (which has been accepted and shouldn't be displayed in this PR any more) is addressing the output problem. GitHub has problem to display that PR too. (On local git, git show only two lines changes but GitHub shows hundreds of lines. Who knows what was happening) I didn't change the output format from int to str in my previous PR. Somebody else did (I didn't run git blame to check who did it) and his commit broke a lot of old code. Sikander couldn't run the simple ks function and asked me for help. I patched the ks function so that it is compatible to str. It is a dirty quick fix, but at least it allow the master branch to work. (Can't imagine how disastrous it is that people use CANA find out master branch cannot work) Some day we need to dig up who and why change the output from int to str and fix codes relying on that.