CIDARLAB / cello

Genetic circuit design automation
http://www.cellocad.org/
BSD 2-Clause "Simplified" License
801 stars 133 forks source link

Fixed bug preventing swaps in simulated annealing algorithm #25

Open jaipadmakumar opened 6 years ago

jaipadmakumar commented 6 years ago

There was a bug in the simulated annealing that prevented the algorithm from performing 'swaps' between gates in a circuit (only 'substitutions' were ever performed).

PrashantVaidyanathan commented 6 years ago

Could you elaborate the steps to re-create the bug, so that we could test it and accept the pull request?

Thanks

jaipadmakumar commented 6 years ago

Yea, sorry. I should've been clearer. I think the easiest way to see this is just to add a print statement or just some basic counter (to see the number of times you hit that if block) right under line 144. You'll see that the if statement is never evaluated as true.

//1. if second gate is used, swap
                if(isNextGateCurrentlyUsed(lc, B_gate)) {
                        <never get here, add print statement or counter here>
...
//2. if second gate is unused, substitute
                else {
                        <always go here>

I think an alternate way if you want a good test case is to use a 2-gate circuit and a 2-gate library. If you do that, you'll see that the circuit never changes b/c the algorithm never performs a swap. The basic problem is that for a given A_gate, you want to disallow groups that are already assigned but allow already assigned gates when running B_gate = getNextGate(). The code then checks later on whether the chosen B_gate is already assigned (performs a swap) or not (performs a substitution). Currently, the function disallows already assigned groups but doesn't specifically allow gates already in the circuit, which prevents the algorithm from ever performing a 'swap'.