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

[BUG]: Exit without throwing an exception #42

Closed
hahnec opened this issue Apr 7, 2022 · 13 comments
Closed

[BUG]: Exit without throwing an exception #42

hahnec opened this issue Apr 7, 2022 · 13 comments
Labels

Comments

@hahnec
Copy link

hahnec commented Apr 7, 2022

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                                                         *
*                                                                             *
===============================================================================
@hahnec hahnec added the Defect label Apr 7, 2022
@KelSolaar
Copy link
Member

KelSolaar commented Apr 7, 2022

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 added a commit that referenced this issue Apr 7, 2022
@KelSolaar
Copy link
Member

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
Copy link
Author

hahnec commented Apr 7, 2022

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
Copy link
Author

hahnec commented Apr 7, 2022

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
Copy link
Member

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
Copy link
Author

hahnec commented Apr 8, 2022

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
Copy link
Member

KelSolaar commented Apr 9, 2022

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 #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
Copy link
Author

hahnec commented Apr 24, 2022

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
Copy link
Author

hahnec commented Jun 26, 2022

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.

@hahnec hahnec reopened this Jun 26, 2022
@KelSolaar
Copy link
Member

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
Copy link
Member

@hahnec
Copy link
Author

hahnec commented Jul 3, 2022

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

@KelSolaar
Copy link
Member

My pleasure!

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

No branches or pull requests

2 participants