Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

auglag: pass ellipsis to gradient #37

Closed
wants to merge 1 commit into from

Conversation

jar1karp
Copy link

This fixes cases where both objective and gradient take more parameters
than just controls and optimization algorithm uses derivatives.

This fixes cases where objective, constraints or their derivatives have
parameters in addition to controls.
@jmh530
Copy link

jmh530 commented Aug 26, 2020

Is there any reason why this has not been merged? I just ran into the issue this PR is supposed to resolve today.

@astamm
Copy link
Owner

astamm commented Jan 28, 2022

Thanks for contributing this PR. I can understand why you would propagate ellipsis to both function and gradient as they probably share the same optional parameters. However, why would you propagate these optional arguments to the constraints?

@jmh530
Copy link

jmh530 commented Jan 28, 2022

@astamm I'm not the original author, it has been some time that I have looked at the issue, and I tend to use other algorithms more frequently than auglag, but sometimes you need to share the optional parameters from the objective/gradient with the constraints. For instance, in a robust portfolio optimization with a variance constraint, the covariance matrix would show up in both the objective/gradients and the constraints. You can rewrite it so that the variance constraint shows up in the objective, but sometimes that's not what you want to do.

@jar1karp
Copy link
Author

jar1karp commented Feb 1, 2022

@astamm It's been a few years, but it's exactly as @jmh530 wrote.

@astamm
Copy link
Owner

astamm commented Feb 1, 2022

Great. I'll give it some more thoughts. I know it was some years ago but I became maintainer only some months ago.

Copy link
Contributor

@aadler aadler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes are no longer appropriate given the changes to the hin/heq directions. Please see my comments below.

@aadler
Copy link
Contributor

aadler commented Jun 26, 2024

Since we started the process of switching the default order of the inequality, the code for all the hin and heq related functions have changed and so this PR should not be merged. If it is critical to use the auglag convience wrapper, we can work on adding the ellipses, but please realize that auglag is just a convenience wrapper to setting up the the full nloptr call (see its source). Is it possible for you to pass the necessary parameters in the full nloptr call, like the following pseudocode? You will have to figure out the appropriate calls to opts, which @hwborchers did for you in writing auglag. You can look at its source code and at the unit test for auglag for examples of a full nloptr invocation.

nloptr(x0,
               eval_f = fn(x, parameters),
               eval_grad_f = gr(x, parameters),
               lb = lower,
               ub = upper,
               eval_g_ineq = hin(x, parameters),
               eval_jac_g_ineq = hinjac(, parameters),
               eval_g_eq = heq(x, parameters),
               eval_jac_g_eq = heqjac(x, parameters),
               opts = opts)

@astamm
Copy link
Owner

astamm commented Jun 27, 2024

I am following @aadler's advice. Please reopen an issue if current version is not satisfying for your use case.

@astamm astamm closed this Jun 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants