search

manoharan-lab / holopy

Hologram processing and light scattering in python
GNU General Public License v3.0
135 stars 50 forks source link

Finish deprecations needed for next release #394

Open briandleahy opened 3 years ago

briandleahy commented 3 years ago

Before the next release, we should finish deprecating

barkls commented 3 years ago

PerfectLensModel model.fit() model.sample()

briandleahy commented 3 years ago

@barkls -- remind me, should we complete remove model.fit() and model.sample(), or leave the fit_warning in place with tips for using the new objects?

barkls commented 3 years ago

My preference would be to remove the fit_warning, but keep placeholder fit() and sample() methods. The placeholders could fail though, with an exception message pointing to the new API, rather than passing with a warning.

On Mon, 17 May 2021 at 11:35, Brian Leahy @.***> wrote:

@barkls https://github.com/barkls -- remind me, should we complete remove model.fit() and model.sample(), or leave the fit_warning in place with tips for using the new objects?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/manoharan-lab/holopy/issues/394#issuecomment-842424056, or unsubscribe https://github.com/notifications/unsubscribe-auth/AETUFJZMFC7EIWHFG676UN3TOEZUFANCNFSM42TUQD4Q .

briandleahy commented 3 years ago

Nmpfit is trickier to deprecate because scipy lmfit does not currently play as well with edge cases (e.g. infinities or more parameters than data points). I'm not sure how much of this is the test suite being problematic, and how much is the scipy lmfit implementation. I will think about a good solution and implement it, but it will take me a little while.

barkls commented 3 years ago

More parameters than data points isn't really an edge case we need to seriously consider - even if lmfit can run in that limit, I think the output will be meaningless.

It's possible the isssue you're running into is forbidden regions of parameter space. If the hologram calculation fails (e.g. overlapping spheres), HoloPy returns a hologram of -np.inf, so then you have an infinity and more parameters than data points. Neither of these cases should really occur under normal operation. It might be ok to change the HoloPy behaviour for what to do when hologram calculation fails to accomodate scipy lmfit.

I'm also happy to keep Nmpfit around for now. It doesn't interact much with the rest of the code so someone else can remove it later without mucking aroud too much in the inner workings of HoloPy.

vnmanoharan commented 10 months ago

Will remove nmpfit in v4, when we redo the inference machinery.