EranOfek / AstroPack

Astronomy & Astrophysics Software Pacakge
Other
16 stars 4 forks source link

`imUtil.psf.fwhm_fromBank` can require unbounded memory or cause all sort of problems #439

Closed EastEriq closed 2 months ago

EastEriq commented 2 months ago

This is the kind of call required by unitCS.focusLoop. The first argument is a simple matrix and not an AstroImage or anything the like.

imUtil.psf.fwhm_fromBank(uint16(rand(9600,6422)*2000),'HalfSize',1000)

It can segfault matlab right away, or cause a report that 1665263517635somethingGB are needed, or complain about several other errors (I'll copy them when I reproduce them, which may happen in the next days as I'm on it).

Please don't dismiss it as "but a realistic image will have the right statistics". Images out of the camera may well be unreal because of any possible defect (out of focus, daytime, capped camera, sensor problems); moreover there should be a path to test the scripts using it in simulation mode. If bad images may cause the function to require unbounded time and resources, that should be trapped early.

EastEriq commented 2 months ago
>> a=uint16(ones(9600,6422)*2000);
>> imUtil.psf.fwhm_fromBank(a,'HalfSize',1000)
Error using matlab.internal.math.histcounts
Requested array exceeds the maximum possible variable size.

Error in imUtil.background.modeVar_LogHist (line 102)
    Nhist = matlab.internal.math.histcounts(LogArray, Edges);

Error in imUtil.psf.fwhm_fromBank (line 120)
            [InPar.Background, InPar.Variance] =
            imUtil.background.modeVar_LogHist(Image);

Related documentation
EastEriq commented 2 months ago
>> a=uint16(rand(9600,6422)*2000+800);
>> imUtil.psf.fwhm_fromBank(a,'HalfSize',1000)
Warning: Rank deficient, rank = 2, tol =  3.193110e+01. 
> In imUtil.background.modeVar_LogHist (line 139)
In imUtil.psf.fwhm_fromBank (line 120)

Error using imUtil.background.modeVar_LogHist (line 154)
Unable to find Var (Var is negative) - need to debug: Mode=2067.140381
Var=-18100.623047

Error in imUtil.psf.fwhm_fromBank (line 120)
            [InPar.Background, InPar.Variance] =
            imUtil.background.modeVar_LogHist(Image);
EastEriq commented 2 months ago
>> imUtil.psf.fwhm_fromBank(a,'HalfSize',1000)
Warning: Rank deficient, rank = 2, tol =  8.063528e+01. 
> In imUtil.background.modeVar_LogHist (line 139)
In imUtil.psf.fwhm_fromBank (line 120)

ans =

   NaN

Need more of them? I can go on.

EastEriq commented 2 months ago
>> a=zeros(9600,6422,'uint16');
>> imUtil.psf.fwhm_fromBank(a,'HalfSize',1000)
Warning: Colon operands must be real scalars. 
> In imUtil.background.modeVar_LogHist (line 115)
In imUtil.psf.fwhm_fromBank (line 120)

Error using matlab.internal.math.histcounts
Requested array exceeds the maximum possible variable size.

Error in imUtil.background.modeVar_LogHist (line 118)
    Nhist = matlab.internal.math.histcounts(Array, Edges);

Error in imUtil.psf.fwhm_fromBank (line 120)
            [InPar.Background, InPar.Variance] =
            imUtil.background.modeVar_LogHist(Image);

Related documentation

As you see the last examples give errors at three different lines of imUtil.background.modeVar_LogHist (102,115 and 139). Errors can be try-catched, but segfaults not.

EranOfek commented 2 months ago

I fixed the code. The logic is that this code is meant for quick and dirty estimates (e.g., focusing the telescope). Therefore, I commented the imUtil.background.modeVar_LogHist that requires an image with Poisson statistics, and instead I am using a two-iteration robust median and variance... I also tested it on real image... [dev1 2ba39ed2]