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

[Feature Request] Option to print progress updates #280

Open
JohnGoertz opened this issue Nov 4, 2019 · 10 comments · May be fixed by #285
Open

[Feature Request] Option to print progress updates #280

JohnGoertz opened this issue Nov 4, 2019 · 10 comments · May be fixed by #285

Comments

@JohnGoertz
Copy link

It would be great to have a 'verbose' option to report the progress of the fit, either through a progress bar a la tqdm or just by printing out the current iteration. I've been trying some very ODE parameter estimations, and it would be nice to have an estimate of how long is left.

@pckroon
Copy link
Collaborator

pckroon commented Nov 6, 2019

Nice idea, but I'm not sure it's feasible. At the end of the day fitting is a minimization problem, and we don't know how many iterations we need to find the nearest local minimum (or even the value of the objective function at that point).
You can try passing something like disp=True or options={disp: True} to Fit.execute. I can't check at the moment. That should make the scipy minimizer print more output.
A similar problem exists for integrating ODEs (the number of steps taken depends on the stiffness of the model, etc. However, you can try to pass printmessg=True to ODEModel. That should make the scipy ODE integrator print more output.
Let me know if those work and provide intelligible output. If they do we can consider wrapping them in a Fit(..., verbose=True) kwarg

@JohnGoertz
Copy link
Author

Well, passing options={'disp': True} to Fit.execute doesn't seem to print anything, at least not for my model, and printmessg=True doesn't provide too much information (it seems to just pass any warnings thrown by odepack).

Even if the algorithm can't estimate how many iterations it has to go, even printing out how many iterations it's done so far, average time per iteration, and perhaps the the change in objective value for each iteration. I'm imagining something akin to the convergence plots in COMSOL, where you can see if it's at least heading in the right direction and how quickly.

@pckroon
Copy link
Collaborator

pckroon commented Nov 21, 2019

I definitely see the value of this. Implementing this is non-trivial though: Minimizers can call Objectives multiple times per iteration/step, e.g. to evaluate gradients. Which means we can't just patch something on Objective.__call__.
We could maybe play with the callback option though, but I'm not looking forward to that.

According to [1] passing disp: True should provide some form of convergence output.

[1] https://docs.scipy.org/doc/scipy/reference/generated/scipy.optimize.minimize.html

@tBuLi
Copy link
Owner

tBuLi commented Nov 25, 2019

The callback is definitely the easiest way to do this since at least it is called only once per iteration, but it might also be scipy specific since other backends might not provide that. Then again, we can cross that bridge when we need to. I agree that changing the objective will probably require a lot of hacking to work. So I'm definitely in favor of leveraging a callback.

Perhaps an API such as:

from symfit.callbacks import FitStatus

fit = Fit(...)
fit_result = fit.execute(callback=FitStatus)

where FitStatus should print something?

@pckroon
Copy link
Collaborator

pckroon commented Nov 25, 2019

Sounds reasonable as a first attempt. The downside is that the callback will only be called with the parameter vector, so FitStatus would also have to know about the Objective. We can wrap this in a sort-of sane way in Fit with a verbose=True flag I think.
I'll mock up something, I'm not going to write more thesis today anyway.

@pckroon pckroon linked a pull request Nov 25, 2019 that will close this issue
@tBuLi
Copy link
Owner

tBuLi commented Nov 25, 2019

It will also get the OptimizeResult state, which contains a lot of additional info. Is there anything you really need the objective for?

@pckroon
Copy link
Collaborator

pckroon commented Nov 25, 2019

Only with trust-constr.

callback: callable, optional
Called after each iteration. For ‘trust-constr’ it is a callable with the signature:
callback(xk, OptimizeResult state) -> bool
where xk is the current parameter vector. and state is an OptimizeResult object, with the same fields as the ones from the return. If callback returns True the algorithm execution is terminated. For all the other methods, the signature is:
callback(xk)
where xk is the current parameter vector.

...
For ‘trust-constr’ it is a callable with the signature:
callback(xk, OptimizeResult state) -> bool
...
For all the other methods, the signature is:
callback(xk)

@tBuLi
Copy link
Owner

tBuLi commented Nov 27, 2019

Why do I continue to get upset about these kinds of inconsistencies? Oh well, I guess we will indeed also have to provide the objective to FitStatus.

The extra evaluation is kind of shit though, since this could impact performance quite a bit. We might have to include a warning to users about this. But I'm also thinking it might be better to decorate the objective in such a way as to add a cache of the last result, so we can retrieve that instead. If we store the current parameter vector + results in this cache, we can simply return the cache if the parameter vector matches the cached one and recalculate otherwise.

@pckroon
Copy link
Collaborator

pckroon commented Nov 27, 2019

Not a terrible idea. The downside is though, that caching will also invoke a performance penalty, which will then hit you always, even if you're running non-verbose. Maybe we could have Fit wrap it's objective in a cache if needed, but that's a strange road to go. I'll play around if I have some time.
There's a few more implementational details though: the last call to Objective might not be at xk, but at a nearby point to estimate a (second) derivative; second, numpy arrays are not hashable, and using them as cache keys is a pain. In addition, since they contain floats (in our case), we depend on the minimizer storing the value rather than reconstructing it (due to float inaccuracies).

@tBuLi
Copy link
Owner

tBuLi commented Nov 27, 2019

I agree with all of the above. Depending on the minimizer this caching might indeed be useless.

But I think decorating the objective only when verbose=True is not that strange actually. And since the objective returns the result anyway, as long as we store it by reference instead of a new copy of the data, it shouldn't cost anything to cache.

The floating error could be a problem but in principle shouldn't be so long as in scipy they use the exact same array to make both calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants