replicahq / doppelganger

A Python package of tools to support population synthesizers
Apache License 2.0
165 stars 32 forks source link

test_balance_cvx_relaxed failing on new versions of cvxpy #57

Open katbusch opened 6 years ago

katbusch commented 6 years ago
    def test_balance_cvx_relaxed(self):
        hh_table, A, w, mu, expected_weights = self._mock_list_relaxed()
        hh_weights, _ = listbalancer.balance_cvx(hh_table, A, w, mu)
        np.testing.assert_allclose(
>           hh_weights, expected_weights, rtol=0.01, atol=0)
E       AssertionError: 
E       Not equal to tolerance rtol=0.01, atol=0
E       
E       (mismatch 100.0%)
E        x: matrix([[ 29.798231],
E               [ 37.155819],
E               [ 55.549788],
E               [157.820203]])
E        y: matrix([[45.],
E               [52.],
E               [65.],
E               [98.]])

test/test_listbalancer.py:220: AssertionError

I've tried downgrading cvxpy, numpy, and pandas to working versions but ran into issues where those older versions are no longer working, so there's not currently a way to get this into a working state

katbusch commented 6 years ago

So it seems this is likely just a better solution. In the old version:

dist(s, K) = 4.5449e-07, dist(y, K*) = 0.0000e+00, s'y/|s||y| = 1.6867e-09
|Ax + s - b|_2 / (1 + |b|_2) = 5.7120e-03
|A'y + c|_2 / (1 + |c|_2) = 1.2275e-02
|c'x + b'y| / (1 + |c'x| + |b'y|) = 1.2320e-05

In the new version

dist(s, K) = 9.2188e-21, dist(y, K*) = 0.0000e+00, s'y/|s||y| = 5.2658e-10
primal res: |Ax + s - b|_2 / (1 + |b|_2) = 5.4125e-06
dual res:   |A'y + c|_2 / (1 + |c|_2) = 2.6497e-07
rel gap:    |c'x + b'y| / (1 + |c'x| + |b'y|) = 9.7344e-09

Error has gone down

katbusch commented 6 years ago

@kaelgreco I've fixed this test by adding two alternate solutions, so this isn't as high priority, but I'm guessing the test is still unstable so please take a look at some point. @bnaul suggested we could check the objective value instead of the weights. There might also be more stable inputs.

bnaul commented 6 years ago

FYI those aren't technically the objective function, they show how much the estimated solution changed in the last iteration (that's how it decides when to stop). The objective looks like

c'x = -282.1634, -b'y = -282.1634

EDIT: checking rel_gap < 1e-6 or so would also be fine