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

NLS: obj, grad! and objgrad! without residual evaluation #429

Closed
tmigot opened this issue Mar 3, 2023 · 7 comments · Fixed by #430
Closed

NLS: obj, grad! and objgrad! without residual evaluation #429

tmigot opened this issue Mar 3, 2023 · 7 comments · Fixed by #430

Comments

@tmigot
Copy link
Member

tmigot commented Mar 3, 2023

There exist functions re-using the residual memory

f = obj(nls, x, Fx)
g = grad!(nls, x, g, Fx)
f, g = objgrad!(nls, x, g, Fx)

However, all of them internally call residual!. It would be great to also have similar functions that don't evaluate the residual.

In a solver context, often we will call residual! and then want to evaluate the objective, gradient, etc.
Any opinion on this @dpo @abelsiqueira @amontoison @geoffroyleconte @paraynaud ?

@abelsiqueira
Copy link
Member

Makes sense. It could be just a keyword argument, what do you think?

@amontoison
Copy link
Member

I like the proposition of Abel, we could add a keyword argument to reuse the value of the Fx vector without calling residual!. If x was not updated, we don't need to call residual! multiple times.

@paraynaud
Copy link
Member

A keyword should be enough, but do you prefer

f = obj(nls, x, Fx; recompute_Fx::Bool=true)

or

f = obj(nls, x; Fx=residual(nls))

while keeping the current f = obj(nls, x, Fx).
I am not sure what is best but neither of the two solutions works without making some changes in the Solvers.

@amontoison
Copy link
Member

The first version with the keyword argument will not break the API. I prefer it.

@tmigot
Copy link
Member Author

tmigot commented Mar 4, 2023

Thanks for the comments. I like @paraynaud first suggestion too. I was wondering about this

obj(nls, x, Fx, shah::UInt64)

with the idea

function obj(nls, x, Fx, shah::UInt64)
  if hash(x) != shah
    return obj(nls, x, Fx) # recompute Fx
  else
    return dot(Fx, Fx) / 2
  end
end

@abelsiqueira
Copy link
Member

I also prefer to not break the current API, so the first suggestion.

Maybe the hash idea could be a CacheNLPModel? Because it makes sense in a few situations (there was talk about it in the past, with the name "memoization"). Just for this case it seems out of place. It would be better to be explicit with a keyword argument like recompute_Fx or skip_residual_call. Also, it might be the case that specific model implementations have a different obj than just the dot(Fx, Fx) / 2, like adding a regularization term, and then it becomes complicated to make sure that you are dispatching correctly.

@dpo
Copy link
Member

dpo commented Mar 4, 2023

Just recompute=true would do it.

The hash idea should be benchmarked carefully. I tried recently and found it slower than comparing vector components one by one even for fairly large n.

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.

5 participants