search

maciej-bendkowski / paganini

Multiparametric tuner for combinatorial specifications
BSD 3-Clause "New" or "Revised" License
5 stars 1 forks source link

FIX: comply with CVXPY's policy regarding * vs. @ #5

Closed Kerl13 closed 4 years ago

Kerl13 commented 4 years ago

See #4

Kerl13 commented 4 years ago

There are still warnings coming from this and this lines. Blindly replacing * by @ makes the test suite pass without a warning by but I suspect something weird is happening here: these are multiplications between two 1D vectors of the same shape which, if I understand correctly, is not a legal operation: we end up in the else clause in this function in cvxpy which says "There might be a dimension mismatch, but we don't check for that here". So I'm a bit lost, what is this multiplication doing? If that's a scalar product maybe transposing one of the two operands would make it clearer?

maciej-bendkowski commented 4 years ago

I was a bit worried too, however having @ everywhere in the code seems to fix the problem. Given that the tests pass once I changed @ in both lines you mention @Kerl13, the @ seems to perform vector multiplication giving an expression (sum in fact), at least theoretically. In practice, np.zeros is not a vector but an array, like cvxpy.Variables. Their dimension seem to mismatch, but, I guess, @ must transpose one of them and do a few other duck typing casts (ehh...). For clarity, we could write

        objective = cvxpy.Maximize(obj @ variables.T)
        problem   = cvxpy.Problem(objective, constraints)
        return self._run_solver(variables, problem, params)

and have the transposition explicit.

Kerl13 commented 4 years ago

You're definitely right: I found this in the numpy documentation (https://numpy.org/devdocs/reference/generated/numpy.matmul.html?highlight=multiply) which backs up you assumption:

If the first argument is 1-D, it is promoted to a matrix by prepending a 1 to its dimensions. After matrix multiplication the prepended 1 is removed. If the second argument is 1-D, it is promoted to a matrix by appending a 1 to its dimensions. After matrix multiplication the appended 1 is removed.

So I guess a regular numpy user (which I am not) would not have been surprised by the 1-D × 1-D product. I explicitly indicated the transposition as you suggested, even if it's a no-op, to make things clearer to people like me ^^

So I think it's done!