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
DM-39733: Specify nan_policy="omit" in model fit #268
Conversation
The tests currently assume that |
371aa94
to
f27c11d
Compare
The problem is that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks OK, with a couple of minor suggestions.
@@ -748,6 +750,11 @@ def dipoleModelFunctor(x, flux, xcenPos, ycenPos, xcenNeg, ycenNeg, fluxNeg=None | |||
if np.any(~mask): | |||
weights[~mask] = 0. | |||
|
|||
# Filter out any nans, and make the weights 0. | |||
nans = (~np.isfinite(z) | ~np.isfinite(weights)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are only worried about NaN
s, I think np.isnan(z) | np.isnan(weights)
would be more clear.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess infinities could be legal? I've changed it.
@@ -748,6 +750,11 @@ def dipoleModelFunctor(x, flux, xcenPos, ycenPos, xcenNeg, ycenNeg, fluxNeg=None | |||
if np.any(~mask): | |||
weights[~mask] = 0. | |||
|
|||
# Filter out any nans, and make the weights 0. | |||
nans = (~np.isfinite(z) | ~np.isfinite(weights)) | |||
z[nans] = 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I try to avoid setting image values to zero, in case there is a non-zero offset. Perhaps z[nans] = np.nanmedian(z)
The lmfit nan_policy needs all the variables to be aligned, but here we are fitting with the "x" grid as the independent variable, but "z" has a different shape. But the nan_policy="omit" option in lmfit simply does a filtering prior to doing any fit, so here we zero out any nans and ensure the weights are 0 for these values.
f27c11d
to
fac7b07
Compare
This was deprecated in lmfit 0.9.8 and removed in 2019.
lmfit/lmfit-py#565