implementing addReluConstraint()

[clazarus]

-implemented method that adds the Relu constraints given two variables
-avoids exposing PiecewiseLinearConstraint to the shared library

[guykatzz]

This file seems almost identical to the other Rules.mk file. I suggest to remove this file and include the usual Rules.mk, and if you need to customize particular commands or add flags, you can do that locally in your Makefile by adding/overriding things

[clazarus]

I have removed this file from this PR and will now include it in the PR with the python wrapper code, sorry for the confusion.

Including this Rules.mk file enabled us to compile the python bindings and get the python module working both in Mac and Ubuntu seamlessly, should we still not include it?

[guykatzz]

No problem about including the delta, but I think most of this file was just copy-paste from the Rules.mk file in the root directory. You can just #include the file from the root directory and locally add the delta that you need. Makefiles allow you to easily override definitions that come from #included files.

[guykatzz]

This violates our current encapsulation: input query doesn't know about particular piecewise linear constraints. It only supports the more general "addPiecewiseLinearConstraint" method. I suggest to remove this new code, and simply have the caller instantiate a ReluConstraint and pass it through the existing interface.

[ShantanuThakoor]

We were trying to avoid exposing the PiecewiseLinearConstraint class to the API, so we wrote this helper function for convenience.
Instead, we could have a function in the MarabouCore.cpp file
void addReluConstraint(InputQuery& ipq, unsigned x1, unsigned x2)
which would add the corresponding Relu constraint to the InputQuery. In this way, the encapsulation would be maintained, and the Python side does not need to know about the general LinearPiecewiseConstraint objects.

Would this be better?

[guykatzz]

The ReluConstraint class is a child class of PiecewiseLinearConstraint, so by exposing ReluConstraint I think we are actually exposing more, not less. The idea is that the entire Marabou core only knows about the more general PiecewiseLinearConstraint class, so that we don't have to make any changes when we add, e.g., max constraints. Currently only the code that populates the input queries knows about the actual sub-classes of PiecewiseLinearConstraint.

I would recommend having a similar abstraction in the python code for easier maintenance and extension, but I won't insist on this :)

[ShantanuThakoor]

Sorry, maybe I was unclear. I did not mean exposing the ReluConstraint class to the API. I meant exposing a function which, given an InputQuery and a pair of variables, adds the corresponding Relu constraint to the InputQuery. Thus, we do not expose the constraint classes themselves to the API, only this function which acts as an interface to add new constraints.

[guykatzz]

Sure, if you want to have such a function in MarabouCore, that's fine.

[ShantanuThakoor]

In that case, we will close this PR and add this functionality to MarabouCore.cpp, which is in a different PR.

[clazarus]

Sorry for the confusion. I mistakenly pushed a commit with all the python code. I have now reverted and will do a separate PR with the python code while we address the comments in this PR.