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

Algorithm should be able to compute "sublosses" efficiently on demand #1986

Open
manchester-jhellier opened this issue Nov 11, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@manchester-jhellier
Copy link
Contributor

Description

Currently, Algorithm has the method get_last_loss. This is fine for algorithms which have a single relevant component to their loss, but leads to unexpected behaviour when used with with some of the more prox-oriented algorithms, as these basically never actually evaluate it. It is also inconvenient when the user wants to know how the different parts of the objective function are changing (if they want to analyse L-curves for example). CGLS has essentially one objective, while for something like PDHG we give it multiple functions to be summed, often corresponding to likelihood and prior components.

I suggest that as an alternative to this we could amend the Algorithm interface to have a get_subobjective(n: int) -> List[float] method, whereby a record is kept of the subobjectives and which iteration they were last evaluated in; there could also be an objective() -> float property which is simply the sum of those (lazily evaluated) subobjectives. This way, we can reuse the calculated value if it's still correct, or trigger evaluation to make sure we yield the fresh result.

The real point to this would be for use in callbacks, so that the user doesn't have to write special callbacks for every different algorithm they want to use. Fairly minimal changes would need to be made to the existing Algorithm implementations to make them compliant with this. I'm happy to make these changes and perform a pull request, but am filing this issue to check that the CIL team doesn't think this is a terrible idea they would reject before I put in the effort.

Environment

24.2.0 None 3.11.10 | packaged by conda-forge | (main, Oct 16 2024, 01:27:36) [GCC 13.3.0] linux

@MargaretDuff
Copy link
Member

Thanks @manchester-jhellier - I think this is complex, because for some algorithms e.g. PDHG we might not ever evaluate the individual functions on the current iterate during the update.

However, each time we call update_objective we normally calculate each sub-function and sum e.g.

def calculate_objective_function_at_point(self, x):
""" Calculates the objective at a given point x
.. math:: f(x) + g(x)
Parameters
----------
x : DataContainer
"""
return self.f(x) + self.g(x)
or
def update_objective(self):
"""Evaluates the primal objective, the dual objective and the primal-dual gap."""
self.operator.direct(self.x_old, out=self.y_tmp)
f_eval_p = self.f(self.y_tmp)
g_eval_p = self.g(self.x_old)
p1 = f_eval_p + g_eval_p
.

Perhaps we could save them at this point, with no extra computational cost other than the storage of a few more numbers.

@casperdcl - do you have opinions on this? We talked about moving the update_objective_interval into a (default) callback?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants