Skip to content

masked pixel leak in _iso_extract #26

Open

Description

@hgao-astro

AutoProf can fail when the input image contains NaNs, even when they are covered by the supplied mask. The mask is read correctly and applied, but _iso_extract() can still return non-finite flux values.

The core issue appears to be in _iso_extract:

  • Flux is sampled/interpolated from the raw image first.
  • The mask is then evaluated only at the rounded sample coordinates.
  • With Lanczos/bicubic interpolation, the interpolation kernel can include nearby NaN pixels.
  • As a result, NaNs can enter downstream FFT, quantile, interpolation, and median calculations.

In principle, this should affect all call sites, e.g., center finding, initialization, isophote fitting, and profile extraction. The leak is not limited to NaN. Pixels that are supposed to be masked now enter the sample and bias the results. NaN happens to be the most catastrophic case and gets caught. My observed failure case was in the center finding step, where Center_HillClimb tried to convert NaN to an integer.

Activity

  1. hgao-astro commented on May 8, 2026

    @hgao-astro
    ContributorAuthor

    I will open a PR soon to fix this.

  2. hgao-astro commented on May 11, 2026

    @hgao-astro
    ContributorAuthor

    Hi @ConnorStoneAstro. When trying to make the interpolation mask-aware, I realize there is no straightforward way to do so for the bicubic interpolation with RectBivariateSpline. The required input of (x, y, z) must be a complete rectangle. The current input for RectBivariateSpline is a rectangle of approximately (2 * Rlim + 10) x (2 * Rlim + 10) centered on the object. I think there are two options to perform a mask-aware interpolation. We can switch to SmoothBivariateSpline, which allows input of scattered points. And we just input the same rectangle minus the masked pixels. Or, we could implement a local cubic-convolution with something like a 4x4 stencil and exclude any extracted samples whose interpolation footprint contains masked pixels. Let me know what you think.

  3. ConnorStoneAstro commented on May 11, 2026

    @ConnorStoneAstro
    Member

    Hi @hgao-astro oh, that's a very nice catch! Yes I had gotten rid of masked pixels in _iso_extract, but the interpolation will cause the NaNs to leak... I think the quick fix is to just use something like the SmoothBivariateSpline you mention to interpolate values for the masked pixels so that they don't leak NaNs any more and dont bias things too much. If we add this as a step early on in the pipeline (it could also automatically add NaNs to the mask since there's nothing else to do with NaN pixels) it would solve any other issues like the center finding algorithm as well. It seems a bit wrong to just fill in guess values for the masked pixels, but some of the algorithms may require a value (like the center of mass center finder), for example if we keep strictly to the mask then the hillclimb algorithm could land somewhere where a isophote is completely masked and it will throw an error rather than carrying on towards the center.

    Your solution for _iso_extract of using SmoothBivariateSpline seems really good to me, but if this leaking is a problem to all the algorithms then it could be a lot of work to get clean solutions for all of them.

  4. hgao-astro commented on May 11, 2026

    @hgao-astro
    ContributorAuthor

    Thanks for the prompt reply @ConnorStoneAstro. At the moment, I am just thinking about how to prevent masked pixels (both NaN and finite) from contaminating the extracted samples via interpolation. For that purpose, I need to know the interpolation footprint, which is easy for the Lanczos interpolation but less obvious for the current bicubic spline interpolation. That is why I was looking for alternatives to RectBivariateSpline to better exclude masked pixels. Once this is achieved, the chances of the whole isophote sample being masked would be higher if extracted via interpolation, therefore the chances of pipeline crash would be higher if one of the steps relies on a non-empty sample.

    I agree that it may not be well justified to replace masked pixel values with some guessed ones. So, I did not mean to suggest interpolation as a preprocessing step, although it may help making the code running more smoothly as a quick fix.

    For now, I would seperate the leaking of masked pixel issue in _iso_extract from the impacts of fixing it on various pipeline steps expecting non-empty samples from _iso_extract and work on the former first. Once it is fixed, I will try running autoprof on examples to reveal the latter and treat them individually.

  5. changed the title [-]NaN pixel leak in `_iso_extract`[/-] [+]masked pixel leak in `_iso_extract`[/+] on May 11, 2026
  6. hgao-astro commented on May 11, 2026

    @hgao-astro
    ContributorAuthor

    I did not explain well enough in the previous comment about the distinction between SmoothBivariateSpline and a local interpolation. The option of SmoothBivariateSpline is close to the original behavior of using RectBivariateSpline, with the same input rectangle but minus the masked pixels. It sounds attractive by allowing to reject masked pixels, but it also shares the property of having a global footprint withRectBivariateSpline. It is difficulty to justify only excluding masked pixels from the interpolation instead of masking the extracted sample if the rectangle has any masked pixels. The latter is impractical since that way few-to-no useful samples left for consumption. However, the former may suffer from extreme mask geometry or asymmetry in galaxy flux distribution. Imagine that left half of the input rectangle is masked and excluded, the extracted samples on the left side will still be valid. But if we use a local interpolation method like lanczos, the left samples will simply dropped.

    At the moment, there are two different interpolation schemes in autoprof: lanczos (local) and bicubic (global). I am leaning towards making the latter local so we can maintain consistent treatment of masked pixels, which is the second option I mentioned before. But if the current global bicubic interpolation is desired for some pipeline steps, we can certainly keep it and write a local one just for _iso_extract.

  7. ConnorStoneAstro commented on May 12, 2026

    @ConnorStoneAstro
    Member

    I think I see what you mean. If we replace the masked pixels with guessed values then sample along the isophote, it may contain a lot of guessed values when we really just want the isophote flux determined by valid values. I think a simple fix for this would just be to identify which samples along the isophote land in a masked pixel and then remove them from the calculation. So the way I see it, this is the path to a solution:

    1- Add a step at the beginning of the pipeline to replace masked pixels with values from SmoothBivariateSpline. Should state some warning/message that it is doing this. This stops other steps from crashing.
    2- In _iso_extract set the new default behaviour as using the SmoothBivariateSpline on valid pixels. If the user changes to one of the other methods and there were masked pixels, include a message that leaking from masked values is possible.
    3- In _iso_extract make sure that isophote sample points that land in a masked pixel are dropped from further calculations. This can be done fairly easily by using pixel coordinates then rounding the values, then converting to integers and querying the mask.

    This way, none of the other methods will crash, and the default _iso_extract behaviour would be purely using valid pixel values. How does that sound to you?

  8. hgao-astro commented on May 22, 2026

    @hgao-astro
    ContributorAuthor

    sorry for the late reply. I have gone down a very different path, because I think it is best to keep masked pixels away from any computation for scientific purposes and not to replace masked pixels with some guessed values. A global interpolation like SmoothBivariateSpline or RectBivariateSpline makes it difficult to implement rejecting interpolated values with footprints over masked pixels.

    1- Add a step at the beginning of the pipeline to replace masked pixels with values from SmoothBivariateSpline. Should state some warning/message that it is doing this. This stops other steps from crashing.

    I prefer not to replace masked values with guesses.

    2- In _iso_extract set the new default behaviour as using the SmoothBivariateSpline on valid pixels. If the user changes to one of the other methods and there were masked pixels, include a message that leaking from masked values is possible.

    That was my original thought. But later, I found it difficult to justify only excluding masked pixels from the interpolation instead of rejecting the interpolated sample. That is what I meant by "a global interpolation like SmoothBivariateSpline or RectBivariateSpline makes it difficult". Besides, since autoprof already use lanzcos interpolation (a local scheme), it is probably better to do a local bicubic interpolation to maintain consistency.

    3- In _iso_extract make sure that isophote sample points that land in a masked pixel are dropped from further calculations. This can be done fairly easily by using pixel coordinates then rounding the values, then converting to integers and querying the mask.

    Such masking was already implemented in the main branch. It was the 2 point that prevents masked pixels from entering the computation via interpolation.

    I admit that my choice involves more comprehensive changes throughout the repo. With the help of the coding agents, I find it is not too difficult to sweep through the pipeline and fix downstream crashes.

    Small tests have shown the fixes are working well. I am giving the code more exercise to reveal any potential issues. Here is a preview of the major changes #27.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions