Refactor Tests #91
Refactor Tests #91
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## gallego-posada/1.0.0-alpha #91 +/- ##
=============================================================
Coverage ? 80.17%
=============================================================
Files ? 28
Lines ? 938
Branches ? 0
=============================================================
Hits ? 752
Misses ? 186
Partials ? 0 ☔ View full report in Codecov by Sentry. |
| def compute_violations(self, x: torch.Tensor) -> cooper.CMPState: | ||
| """ | ||
| Computes the constraint violations for the given parameters. | ||
| """ |
There was a problem hiding this comment.
Add a comment that sampling for strict violation and violation is different
tests/test_pipeline.py
Outdated
| self.device = device | ||
|
|
||
| def test_convergence(self, extrapolation, alternation_type): | ||
| x = torch.nn.Parameter(torch.ones(5, device=self.device)) |
There was a problem hiding this comment.
start from random init
merajhashemi
force-pushed
the
merajhashemi/refactor-tests
branch
from
0b32859 to
c49f65d
Compare
Remove constraint_type from multiplier init. Add attribute to check if module expects features.
cooper/formulations/formulations.py
Outdated
| if self.expects_penalty_coefficient and penalty_coefficient is None: | ||
| raise ValueError(f"{type(self).__name__} expects a penalty coefficient but none was provided.") | ||
| if not self.expects_penalty_coefficient and penalty_coefficient is not None: | ||
| raise ValueError(f"{type(self).__name__} does not expect a penalty coefficient but one was provided.") |
There was a problem hiding this comment.
Currently this has code repetition with Constraint.sanity_check_penalty_coefficient. Let's merge the two.
Create a helper function for this in the Formulation called sanity_check_penalty_coefficient and call it here. Moreover, replace the check in Constraint with a self.formulation.sanity_check_penalty_coefficient
There was a problem hiding this comment.
Done in 3ee14f9
| def compute_violations(self, x: torch.Tensor) -> cooper.CMPState: | ||
| """ | ||
| Computes the constraint violations for the given parameters. | ||
| """ |
| constraints = [] | ||
| if self.has_ineq_constraint: | ||
| constraints.append(self.A.cpu().numpy() @ x <= self.b.cpu().numpy()) | ||
| if self.has_eq_constraint: | ||
| constraints.append(self.C.cpu().numpy() @ x == self.d.cpu().numpy()) |
There was a problem hiding this comment.
Do we ever use both equalities and inequalities in our tests? Would the code work then?
Otherwise, can we raise an error if someone tries having both?
There was a problem hiding this comment.
No. We don't currently have any tests that have both types of constraints. But the cmp should work 🤔
No description provided.