Added test for infeasible allocation and single balance_cvx with rela…

[kaelgreco]

Added test for infeasible allocation and single balance_cvx with relaxations. Narrowed tolerances and use better checks on 'trust controls check' and 'trust initial check' Fix for broadcast error on zero marginals

Fixed np.insert bug in balance_multi_cvx and zero weight solver error in discretize_multi_weights (np.insert will insert one value along given axis at boundary (single append), but won't handle multiple values. Now differentiates inserts and appends, and a test has been modified to test this.

Added protection against infeasible solution in household weight discretization (just rounds residuals and returns) @alexeisw : let me know if this is the right approach in your eyes.

---

This change is 

[katbusch]

A few comments on style & error logging

---

Review status: 0 of 2 files reviewed at latest revision, 3 unresolved discussions.

---

doppelganger/listbalancer.py, line 179 at r1 (raw file):

zs_out = np.insert(z.value, zero_marginals_insert, np.zeros((n_controls, 1)), axis=1)

This and some lines above should be able to go on one line now. You might want to check that all your linting tools allow 100 chars instead of 80

---

doppelganger/listbalancer.py, line 257 at r1 (raw file):

    except cvx.SolverError:
        logging.info(

How about logging.exception instead of logging.info? That will print out the exception.

---

doppelganger/listbalancer.py, line 264 at r1 (raw file):

    # Insert zeros
    if zero_weights_inds.size:
        weights_filter = zero_weights_inds < weights_out.shape[0]

This code looks pretty similar to the zeros code in balance_multi_cvx. Is it possible to factor them out into a shared function that you don't need to remember to make changes/fixes to both?

---

Comments from Reviewable

[alexeisw]

Review status: 0 of 2 files reviewed at latest revision, 5 unresolved discussions, all commit checks successful.

---

doppelganger/listbalancer.py, line 237 at r1 (raw file):

        cvx.sum_entries(
            cvx.sum_entries(cvx.mul_elemwise(x_log, y), axis=1) -
            (gamma - 1) * cvx.sum_entries(U, axis=1) -

There is no need for ( gamma - 1 ), can be replaced with gamma.

---

doppelganger/listbalancer.py, line 260 at r1 (raw file):

            'Solver error encountered in weight discretization. Weights will be rounded.')

    weights_out = y.value if np.any(y.value) else x_residuals

x_residuals contain nearest lowest integers (i.e. 0.777 gets rounded to 0) due to the problem formulation needs. I'd suggest using np.rint(x) as a backup solution if solver fails

---

Comments from Reviewable

[katbusch]

Review status: 0 of 2 files reviewed at latest revision, 6 unresolved discussions, all commit checks successful.

---

doppelganger/listbalancer.py, line 11 at r2 (raw file):

def insert_append(arr, indices, values, axis=0):

Nice! So clean. Looks like this is only used in this file so convention is to name you could name it _insert_append to show that it's private

---

Comments from Reviewable

[kaelgreco]

doppelganger/listbalancer.py, line 11 at r2 (raw file):

Previously, katbusch (Kat Busch) wrote…

Nice! So clean. Looks like this is only used in this file so convention is to name you could name it `_insert_append` to show that it's private

you're right! thanks

---

Comments from Reviewable

[kaelgreco]

Review status: 0 of 2 files reviewed at latest revision, 6 unresolved discussions, some commit checks pending.

---

doppelganger/listbalancer.py, line 179 at r1 (raw file):

Previously, katbusch (Kat Busch) wrote…

> zs_out = np.insert(z.value, zero_marginals_insert, > np.zeros((n_controls, 1)), axis=1) This and some lines above should be able to go on one line now. You might want to check that all your linting tools allow 100 chars instead of 80

fixed. thanks!

---

doppelganger/listbalancer.py, line 237 at r1 (raw file):

Previously, alexeisw wrote…

There is no need for ( gamma - 1 ), can be replaced with gamma.

fixed.

---

doppelganger/listbalancer.py, line 257 at r1 (raw file):

Previously, katbusch (Kat Busch) wrote…

How about `logging.exception` instead of `logging.info`? That will print out the exception.

Good call. Done.

---

doppelganger/listbalancer.py, line 260 at r1 (raw file):

Previously, alexeisw wrote…

x_residuals contain nearest lowest integers (i.e. 0.777 gets rounded to 0) due to the problem formulation needs. I'd suggest using np.rint(x) as a backup solution if solver fails

I actually do the round on the return: np.array(weights_out > 0.5).astype(int) Or am I misunderstanding?

---

doppelganger/listbalancer.py, line 264 at r1 (raw file):

Previously, katbusch (Kat Busch) wrote…

This code looks pretty similar to the zeros code in `balance_multi_cvx`. Is it possible to factor them out into a shared function that you don't need to remember to make changes/fixes to both?

Yeah, good suggestion! I made a separate function which cleans things up a bit.

---

Comments from Reviewable

[alexeisw]

Review status: 0 of 2 files reviewed at latest revision, 6 unresolved discussions, all commit checks successful.

---

doppelganger/listbalancer.py, line 260 at r1 (raw file):

Previously, kaelgreco wrote…

I actually do the round on the return: `np.array(weights_out > 0.5).astype(int)` Or am I misunderstanding?

Yep, in case the solver fails we'd like to return np.rint(x)

x_residuals are not values rounded to the nearest integer, they are residuals.

---

Comments from Reviewable

[alexeisw]

LGTM rounding behavior is correct!

---

Review status: 0 of 2 files reviewed at latest revision, 6 unresolved discussions, all commit checks successful.

---

Comments from Reviewable