Avoid model recreation at every call of reluplex_step

[castrong]

Currently, the model is recreated whenever a ReLU is fixed. This changes it so that (1) the desired constraints are added to the model to fix the ReLU to be active or inactive, (2) the recursive call is made, then (3) the constraints are removed from the model. This is now possible b/c of added functionality to the JuMP optimization library.

[tomerarnon]

Thanks for putting together this PR @castrong!
In addition to Mykel's comment about debug code, I think there is even greater opportunity for simplification once this major change is made. For example, I think the relu_status variable becomes obsolete in reluplex_step after this change, and can therefore be removed. As a result, the loop on 1:2 can even become a loop over for repair in (type_one_repair, type_two_repair). I even think the entire function even goes away! This really simplifies the algorithm's look a lot.

[castrong]

@tomerarnon I was considering keeping relu_status around for debugging purpose but it would also be cleaner without. I'll try to implement some of the changes you suggested - I'm not sure I get what you mean by the entire function goes away? Could you elaborate on that a bit more?

Thanks!
Chris

[tomerarnon]

It was a typo! I meant the entire enforce_repairs function goes away.

[tomerarnon]

By the way, it seems the tests are failing currently because con_one/con_two can't be reassigned using that syntax.
The error says:
An object of name con_one is already attached to this model. If this is intended, consider using the anonymous construction syntax, e.g., x = @variable(model, [1:N], ...) where the name of the object does not appear inside the macro.

[castrong]

Ok I tried changing that and refactored the for loop to be cleaner as suggested :) - I'll see if it breaks any tests now

[tomerarnon]

Looks like that fixed it!

One final stylistic suggestion before merging: how about instead of con_one/two, having something like

new_constraints = repair!(...)
...
delete.(model, new_constraints)

[castrong]

sounds great - just pushed that - also should I be worried about the coverage/coveralls check?

[tomerarnon]

just pushed that

Awesome. We already know it works, but I'll wait for the tests out of paranoia anyway.

should I be worried about the coverage/coveralls check

Nope. <1% in either direction isn't meaningful anyway.