peri-source / peri

Parameter Extraction from Reconstructing Images
https://peri-source.github.io/peri-docs/
MIT License
9 stars 7 forks source link

Catching invalid updates #3

Open briandleahy opened 7 years ago

briandleahy commented 7 years ago

Right now objs.PlatonicSpheresCollection raises an error when a particle is placed well outside the box.

For instance:

import numpy as np
from peri import states, models, util
from peri.comp import objs
o = objs.PlatonicSpheresCollection(np.ones([1,3]), np.array([5]))
im = util.NullImage(shape=(10,10,10))
s = states.ImageState(im, [o], mdl=models.ParticlesOnlyModel(), model_as_data=True)
s.update('sph-0-z', 9001.0)
# doesn't raise an error, but:
s.obj_remove_particle(0)
# raises UpdateError: update triggered invalid tile size

This makes it difficult to use a generic optimization scheme -- if an optimizer decides that a particle should be well outside the image (i.e. removed) then things can break.

One solution is to clip all updates to certain values -- e.g. on an update restrict to o.shape.shape - o.inner.l or something. This is nice but the user doesn't know if their

Another is to raise an error when attempted bad updates. Nice for the user but doesn't solve the problem for an optimizer.

A third which I like the best is both -- raise an error as default, allow a boolean flag to be passed to update which catches errors. This would have to be in all the Component updates, and would have to do nothing for the ones that don't need to clip values.

A fourth is to have the object and/or state know what an OK value is and somehow pass it back to the optimizer, which clips to some value. I don't really like this one because it seems a little messy.

By the way, there is already something like this in exactpsf, which clips alpha, n2n1, etc to be reasonable values (with a warn) if a bad update is attempted.

Tagging @mattbierbaum because I want to hear his thoughts.

mattbierbaum commented 7 years ago

Options that I can think of (and yours) in a list:

1) A c-style return value of True or False from update which says whether the method succeeded in updating the component. Some components already implement this but currently it is not respected by other methods. If we go this route, we would need to add additional functionality to ComponentCollection update which knows how to roll back updates to some Components if one of them ends up failing since not doing so could leave it in an inconsistent state.

2) All Components know about constraints for parameter values and create an interface for other things (optimizers) to ask about them. This really isn't so messy since everybody is still only responsible for themselves. The state would just be the intersection of these limits. (Could this be implemented as priors?)

3) Raise an exception when a bad step is taken. I don't understand the boolean flag passed to update? Is that the same as (1)? Also probably need to deal with possible inconsistent states.

Any more for the list?

briandleahy commented 7 years ago

Envisioning something like this for a boolean flag passed to update:

def update(self, params, values, docorrect=False):
    if docorrect:
        values[:] = np.clip(values, minvals, maxvals)
    if np.any(values == bad):
        raise ValueError
    # do rest of update
briandleahy commented 6 years ago

(This is the problem with the assertion error on check_groups).