colour-science / colour-demosaicing

CFA (Colour Filter Array) demosaicing algorithms for Python
https://www.colour-science.org
BSD 3-Clause "New" or "Revised" License
285 stars 58 forks source link

[BUG]: Exit without throwing an exception #42

Closed hahnec closed 2 years ago

hahnec commented 2 years ago

Description

Hey folks,

I am a frequent user of your library and made it even part of my own project PlenoptiCam, where I recently encountered a problem, which I believe is due to the colour-demosaicing software library. Raw images passed to PlenoptiCam are relativey large in terms of their spatial resolution, thus requiring a large amount of memory. I observed that occasionally my software quit at some point without throwing an error or exception. When debugging, I noticed that this behaviour occurs at a point of my pipeline where the rather costly demosaicing_CFA_Bayer_Menon2007 function is called. I attempted to reproduce the problem with a synthetic image having a very large image resolution and observed the same problem: program abort without any message. Below you can find a short code example causing this issue on my machine.

While it is obvious why this error occurs, I would like to have a little control about the case when it happens. Would it be possible to throw an exception just before the function call crashes? This would allow users to keep programs running and let them know about the cause of the problem.

Code for Reproduction

import numpy as np
from colour_demosaicing import demosaicing_CFA_Bayer_Menon2007

mockup_image = np.ones([int(1e4), int(1e4)])
res = demosaicing_CFA_Bayer_Menon2007(CFA=mockup_image, pattern="RGGB", refining_step=True)

Exception Message

An Exception would be desired, but unfortunately there is no.

Environment Information

===============================================================================
*                                                                             *
*   Interpreter :                                                             *
*       python : 3.8.10 (default, Mar 15 2022, 12:22:08)                      *
*                [GCC 9.4.0]                                                  *
*                                                                             *
*   colour-science.org :                                                      *
*       colour : 0.4.1                                                        *
*       colour-demosaicing : 0.2.1                                            *
*                                                                             *
*   Runtime :                                                                 *
*       imageio : 2.16.1                                                      *
*       matplotlib : 3.5.1                                                    *
*       networkx : 2.7.1                                                      *
*       numpy : 1.22.3                                                        *
*       scipy : 1.8.0                                                         *
*                                                                             *
===============================================================================
KelSolaar commented 2 years ago

Hi @hahnec,

Thanks for reporting the issue! If the process is producing a segmentation fault, there is not much we can do about it.

I managed to run your code without issue here which allowed me to see that about 16-18Gb of RAM was used, it peaked at around 12.5Gb without the refining step. If you look at the code, we are already cleaning up forcibly a lot of allocated variables to lower memory pressure.

If you don't mind loosing precision, you could set colour precision to 32bit before your calls: colour.utilities.set_default_float_dtype(np.float32) This should lower memory usage further. I will see if all the array creation are using colour functions to ensure that the set precision is used across the board.

Cheers,

Thomas

KelSolaar commented 2 years ago

If you use the colour.utilities.set_default_float_dtype(np.float32) call above with the latest from the develop branch, the memory usage should be roughly divided by two, computations speed will also increase quite a bit. Let me know how it goes!

hahnec commented 2 years ago

Hey Thomas, thanks for your suggestion to go for the memory-efficient way of using the function. I will give it a try and adapt my code. Meanwhile, I was thinking of using your less-memory consuming de-bayering functions as a workaround while sacrificing on quality. To trigger the issue on your side, you could simulate the RAM limit by further increasing the image size to something like:

mockup_image = np.ones([int(4e4), int(4e4)])

Given that 16GB is pretty much common standard these days, it also means that there is always an exception to that. Not knowing what has caused a tool to crash is a bit unsatisfying on my side. Hence, my ideal solution would give the memory-expensive method a chance using a try-except statement and inform the user about the memory problem if it occurs. So, I still wonder though whether it is possible to throw/catch an exception?

hahnec commented 2 years ago

Digging further into this, I noticed the crash occurs in masks.py at line 78 which looks like this

channels[channel][y::2, x::2] = 1

It is a harmless numpy index assignment. So, throwing an exception is rather up to the numpy lib I guess. Initializing the channel variable as numpy bool early on (line 76) helps in saving memory, but still causes my call to be killed for a large frame. I probably have to stick to the above-mentioned workarounds for now and hope memory limits will not be exceeded. Thanks anyway!

KelSolaar commented 2 years ago

It is a harmless numpy index assignment. So, throwing an exception is rather up to the numpy lib I guess. Initializing the channel variable as numpy bool early on (line 76) helps in saving memory, but still causes my call to be killed for a large frame. I probably have to stick to the above-mentioned workarounds for now and hope memory limits will not be exceeded. Thanks anyway!

Another option would be to invoke the demosaicing process in a sub-process, this is not great nor ideal from an implementation standpoint but it would give you an opportunity to wrap the entire execution AND catch any problems whilst not crashing the main process.

hahnec commented 2 years ago

Spawning a sub-process (or even a new thread?) is an interesting way to prevent the function from being killed without exceptions. Why do you think it is not great nor ideal?

With PR #46 I propose to initialize masks directly as boolean to lower the memory footprint. The for-loop at the return statement then becomes redundant, which is a nice side effect and reads better in my view. Tests successfully passed. If you don't have any objection, I would be happy to have this feature in a future version as it further reduces the chances of out-of-memory issues.

KelSolaar commented 2 years ago

Why do you think it is not great nor ideal?

You need to maintain some sort of wrapper to spin-up the process, do the demosaicing, save the image, and then read the demosaiced image in the parent process instead of doing it directly. It is rather painful!

With PR https://github.com/colour-science/colour-demosaicing/pull/46 I propose to initialize masks directly as boolean to lower the memory footprint.

Thanks, this is great and how the code should have been in the first place!

Cheers,

Thomas

hahnec commented 2 years ago

Closing as the root of this issue is not related to this repo. Thanks for accepting the PR. Looking forward to the new release!

hahnec commented 2 years ago

Is there a date, when a new release of this repo featuring the above PR can be expected? As can be seen here, users of PlenoptiCam still face problems addressed in this issue. I would appreciate a new release with my commits as it lowers the risk of running into memory issues.

KelSolaar commented 2 years ago

Hi @hahnec,

I generally release a new version along side a main Colour release or if people ask for it, which you did! I will look at that over the weekend. If I forget, entirely possible, please poke a stick at me until it is done!

Cheers,

Thomas

KelSolaar commented 2 years ago

Hello @hahnec,

This has been released!

Cheers,

Thomas

hahnec commented 2 years ago

It's much appreciated Thomas! I will then also work on a new release on my side! Many thanks!

KelSolaar commented 2 years ago

My pleasure!