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

Comments #3

Closed
7 tasks done
dpo opened this issue Aug 30, 2019 · 3 comments
Closed
7 tasks done

Comments #3

dpo opened this issue Aug 30, 2019 · 3 comments

Comments

@dpo
Copy link
Member

dpo commented Aug 30, 2019

I haven't yet tried to run your code but here are a few comments and suggestions from a first reading:

  • obj() allocates a new vector every time for the constraint values; why not preallocate a vector of size ncon inside the AugLagModel and use cons!() instead?
  • obj() and grad() both evaluate the constraints, so you probably want to implement objgrad!() and save one constraint evaluation;
  • we should probably implement memoization so grad() does not reevaluate the constraints where obj() just evaluated them;
  • hess_structure!() doesn't look right; the J'J term could fill in the Hessian substantially. It's not hard to figure out the sparsity pattern of J'J from that of J;
  • hess_coord!() doesn't look right either. Also it should call hess_coord!() and jac_coord() on the original nlp;
  • there is no need to implement grad(), hess(), hess_structure(), hess_coord() and hprod() because you already implement the ! version of those methods;
  • there is no need to implement hess_op() and hess_op!() because you already implement hprod!().

Note that because you use TRON as subproblem solver, you don't need hess_structure!() or hess_coord!(). You could leave those out for now.

@Egmara
Copy link
Contributor

Egmara commented Sep 18, 2019

  • Unnecessary functions have been removed;

  • here replaces cons function #15 I used the pre-allocation for constraint values in AugLagModel and cons!() instead cons() in obj(), grad!() and hprod!() functions. But, I dont implement memoization yet;

  • The objgrad!() function for AugLagModel is not required in the inner solver (tron), do I need to implement this?

@dpo
Copy link
Member Author

dpo commented Oct 25, 2019

Is it feasible to modify TRON to use objgrad()? It seems like we would save quite a bit here.

@abelsiqueira
Copy link
Member

I've created JuliaSmoothOptimizers/JSOSolvers.jl#30 to handle that.

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 a pull request may close this issue.

3 participants