54 multiplier models #66
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Black reformatted
- Create MultiplierModel as instance of multipliers instead of multiplier_model class - Implement MultiplierModel tests
Added MultiplierModel
Added LagrangianModelFormulation
Fix imports
Change meta to metaclass
daoterog
suggested changes
Dec 8, 2022
| Lagrange multipliers. | ||
| is_positive: Whether to enforce non-negativity on the values of the | ||
| multiplier. | ||
| """ |
There was a problem hiding this comment.
Are these arguments valid? I think that they should be removed and we should make explicit that class is meant to be inherited by the model class, just as it is done with torch.nn.Module
| @@ -119,6 +124,11 @@ def load_state_dict(self, state_dict: dict): | |||
| """ | |||
| pass | |||
|
|
|||
| def flip_dual_gradients(self): | |||
| class LagrangianModelFormulation(BaseLagrangianFormulation): | ||
| """ | ||
| # TODO: document this | ||
| Computes the Lagrangian based on the predictions of a `MultiplierModel`. |
There was a problem hiding this comment.
Give a more complete description of the formulation and add the arguments of the init to it.
|
|
||
| # Update multipliers based on current constraint violations (gradients) | ||
| self.dual_optimizer.step() | ||
|
|
||
| if self.formulation.ineq_multipliers is not None: | ||
| # TODO: Check if the isintance flag has to be added to other optimizers |
| @@ -65,6 +65,8 @@ def base_sanity_checks(self): | |||
| Perform sanity checks on the initialization of ``ConstrainedOptimizer``. | |||
| """ | |||
|
|
|||
| # TODO: ensure that the dual optimizer and the dual scheduler is initialized when LagrangeModelFormulation | |||
| @@ -94,17 +94,20 @@ def step( | |||
|
|
|||
| def dual_step(self): | |||
|
|
|||
| # TODO: Fill the flip dual gradients and model formulation flag with other optimizers | |||
| # It's unclear why this difference exists. Must be checked before | ||
| # merging into main | ||
| # new_loss = lagrangian - self.accumulated_violation_dot_prod | ||
| # new_loss.backward(inputs=dual_vars) |
There was a problem hiding this comment.
This alternative implementation should be disregarded, that implementation corresponds to confusion made while looking at the authors' code, it was a test. The current implementation is the one that works and should be kept.
| self.eq_multiplier_model = eq_multiplier_model | ||
|
|
||
| if self.ineq_multiplier_model is None and self.eq_multiplier_model is None: | ||
| # TODO: document this |
There was a problem hiding this comment.
Is it really necessary to document this?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #54
Changes
On the multipliers module we created a new class called cooper.multipliers.MultiplierModel, which predicts the value of the Lagrange multipliers associated with the equality or inequality constraints of a cooper.problem.ConstrainedMinimizationProblem.
Additionally, we created a new Lagrangian formulation --> cooper.formulation.LagrangianFormulation that works with the created
MultiplierModelclass.Testing
We tested the class multipliers.MultiplierModel with the simplest model. A linear model with a single output.
The new Lagrangian Formulation was also tested.
References
This idea comes from the paper by Narasimhan et al. and we took their repository as reference.