Skip to content
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

FlatStruct is extremely inefficient #392

Open
Cybis320 opened this issue Aug 27, 2024 · 6 comments
Open

FlatStruct is extremely inefficient #392

Cybis320 opened this issue Aug 27, 2024 · 6 comments
Assignees

Comments

@Cybis320
Copy link
Contributor

I'm testing use_flat: true on a 1080p capture. Flux is unable to proceed because it runs out of memory (All 32 GB of RAM and 18 GB of swap!). Looking into it, the FlatStruct class is creating multiple Float64 copies of the image array from the flat.bmp. It takes a 2MB images and produces a 33MB object. This breaks the pickling pipeline.
How widely used is flat? Is Float64 necessary anywhere? Is it worth rewriting the FlatStruct class?

@dvida
Copy link
Contributor

dvida commented Aug 27, 2024

Agreed, it should be rewritten.

The flat should note be operationally used with RMS cameras. The vignetting coefficient takes care of everything needed for these wide-field lenses, and making a truly good flat file is very tricky anyway (it's not the automatically generated flat.bmp).

@Cybis320
Copy link
Contributor Author

Right, I was confused by that. A true flat should be produced in a controlled environment with an extremely uniform surface. What is the auto generated flat then?
Should use_flat even be in config?

@dvida
Copy link
Contributor

dvida commented Aug 27, 2024

The terminology has been adopted from another professional system, where making a flat this way really results in a good flat. The GMN "flats" are mostly used to understand the sky brightness and locate obstructions.

If used with very narrow lenses and under dark skies, good flats would be produced by the current method.

@Cybis320
Copy link
Contributor Author

That makes sense on long lenses. I probably can't get to refactor it real soon.
Opinion on float64? Necessary?

@dvida
Copy link
Contributor

dvida commented Aug 29, 2024

Float is necessary for the computations because the levels are multiplied and divided. However, outside the places where calculations are done, it can be stored as an integer.

However, it is very important to check everywhere in the codebase what is happening to the flat and if there are any assumptions anywhere about its type.

@Cybis320
Copy link
Contributor Author

Float is necessary for the computations because the levels are multiplied and divided. However, outside the places where calculations are done, it can be stored as an integer.

However, it is very important to check everywhere in the codebase what is happening to the flat and if there are any assumptions anywhere about its type.

Thanks Denis, I'll keep looking into it. The FlatStruct object currently stores two float64 image arrays: flat_img and flat_img_raw. However, only flat_img is ever needed outside the class definition and it's only used by the applyFlat function and in skyfit but just to check the flat shape. So, right there it seems it's already twice the size it needs to be.

The flat and dark images are stored as bmp (only in 8-bit I think? but the code is meant to handle higher bit as needed), so we are working of quantized data to begin with. I understand the desire to keep the precision after operating on 8-bit data but 64-bit is probably overkill.

It looks like the only thing using FlatStruct.flat_img is applyFlat. So the FlatStruct object is applied to a low bit img and the applyFlat returns a low bit img.

At a minimum, only a single array should be stored. And that single array should probably only be 16 or 32 bit.

I updated the PR #393 to make all the calculations in float32 if dealing with 8-bit data, and float64 if dealing with 16-bit data. The FlatStruct object now only store one array (either 32 or 64 bit) and the array is the inverse of the flat with the dark applied for faster calculation in applyFlat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants