mabuchilab / Instrumental

Python-based instrumentation library from the Mabuchi Lab.
http://instrumental-lib.readthedocs.org/
GNU General Public License v3.0
120 stars 80 forks source link

maximum recursion depth exceeded #94

Open tang37 opened 5 years ago

tang37 commented 5 years ago

Hi,

I am trying to use your code to drive Thorlabs DCC1545M in python3, but it gave me error like following:

RecursionError                            Traceback (most recent call last)
<ipython-input-7-2445f93d9646> in <module>
      1 cv2.destroyAllWindows()
----> 2 mycam.close()

~\Anaconda3\lib\site-packages\instrumental_lib-0.6.dev0-py3.7.egg\instrumental\drivers\cameras\uc480.py in close(self)
    704     def close(self):
    705         """Close the camera and release the associated image memory."""
--> 706         self._dev.ExitEvent(lib.SET_EVENT_SEQ)
    707         self._dev.ExitEvent(lib.SET_EVENT_FRAME)
    708 

~\Anaconda3\lib\site-packages\nicelib\nicelib.py in __call__(self, *args)
    691 
    692     def __call__(self, *args):
--> 693         return self._libfunc(*(self._niceobj._handles + args), niceobj=self._niceobj)
    694 
    695 

~\Anaconda3\lib\site-packages\nicelib\nicelib.py in __call__(self, *args, **kwds)
   1151             'funcargs': c_args,
   1152         }
-> 1153         return self.sig.extract_outputs(c_args, retval, ret_handler_args)
   1154 
   1155 

~\Anaconda3\lib\site-packages\nicelib\nicelib.py in extract_outputs(self, c_args, retval, ret_handler_kwargs)
    212 
    213         if self.ret_handler:
--> 214             retval = self.ret_handler.handle(retval, ret_handler_kwargs)
    215 
    216         if retval is not None:

~\Anaconda3\lib\site-packages\nicelib\nicelib.py in handle(self, retval, available_kwargs)
    535             raise KeyError("Unknown arg '{}' in arglist of ret-handling function "
    536                            "'{}'".format(e.args[0], self.__name__))
--> 537         return self.__func__(retval, **kwargs)
    538 
    539 

~\Anaconda3\lib\site-packages\instrumental_lib-0.6.dev0-py3.7.egg\instrumental\drivers\cameras\uc480.py in ret_cam_errcheck(result, niceobj)
    109 def ret_cam_errcheck(result, niceobj):
    110     if result != NiceUC480.SUCCESS:
--> 111         err_code, err_msg = niceobj.GetError()
    112         raise UC480Error(code=result, msg=err_msg)
    113 

... last 5 frames repeated, from the frame below ...

~\Anaconda3\lib\site-packages\nicelib\nicelib.py in __call__(self, *args)
    691 
    692     def __call__(self, *args):
--> 693         return self._libfunc(*(self._niceobj._handles + args), niceobj=self._niceobj)
    694 
    695 

RecursionError: maximum recursion depth exceeded

I was able to get the first frame but after the first frame, i got the error no matter what I run. I am using python3 on windows7. nicelib version is 0.6, and instrumental version is 0.6.dev0

Appreciate it for any help! Yu

natezb commented 5 years ago

I'm not 100% sure, but I have a guess. You may be closing the camera after it's already been closed. If the camera handle were not currently valid, the call to ExitEvent() would return an error code. This error code is then looked up using GetError(), which requires a valid handle to work. It returns a similar error code, and the recursion begins.

So, I'm guessing that's your underlying problem. NiceLib should probably have a way of avoiding this recursion issue, or at the very least the Instrumental driver should work around it so you get a more useful error message. That's something I'll have to take a look at.

tang37 commented 5 years ago

Thank you for the quick reply! I indeed tried to close the camera within a loop. Now I moved the camera close out, and the problem solved.

developedby commented 4 years ago

I'm having the same problem, except not when double closing. I'm closing the camera, creating a new object with uc480.UC480_Camera() and then trying to do a start_live_video()

File "C:\Users\SAO\Documents\sao\sao\vision\camera.py", line 49, in open
  self.hcam.start_live_video(exposure_time='80.0ms')
File "<decorator-gen-2>", line 2, in start_live_video
File "C:\Users\SAO\AppData\Local\Programs\Python\Python37\lib\site-packages\instrumental_lib-0.6.dev0-py3.7.egg\instrumental\drivers\util.py", line 339, in wrapper
  result = func(*new_args, **new_kwargs)
File "C:\Users\SAO\AppData\Local\Programs\Python\Python37\lib\site-packages\instrumental_lib-0.6.dev0-py3.7.egg\instrumental\drivers\cameras\uc480.py", line 826, in start_live_video
  self._set_binning(kwds['vbin'], kwds['hbin'])
File "C:\Users\SAO\AppData\Local\Programs\Python\Python37\lib\site-packages\instrumental_lib-0.6.dev0-py3.7.egg\instrumental\drivers\cameras\uc480.py", line 767, in _set_binning
  self._dev.SetBinning(mode)
File "C:\Users\SAO\AppData\Local\Programs\Python\Python37\lib\site-packages\nicelib\nicelib.py", line 693, in __call__
  return self._libfunc(*(self._niceobj._handles + args), niceobj=self._niceobj)
File "C:\Users\SAO\AppData\Local\Programs\Python\Python37\lib\site-packages\nicelib\nicelib.py", line 1153, in __call__
  return self.sig.extract_outputs(c_args, retval, ret_handler_args)
File "C:\Users\SAO\AppData\Local\Programs\Python\Python37\lib\site-packages\nicelib\nicelib.py", line 214, in extract_outputs
  retval = self.ret_handler.handle(retval, ret_handler_kwargs)
File "C:\Users\SAO\AppData\Local\Programs\Python\Python37\lib\site-packages\nicelib\nicelib.py", line 537, in handle
  return self.__func__(retval, **kwargs)
File "C:\Users\SAO\AppData\Local\Programs\Python\Python37\lib\site-packages\instrumental_lib-0.6.dev0-py3.7.egg\instrumental\drivers\cameras\uc480.py", line 78, in wrap

This ends with an infinite recursion. I'll have a look at nicelib later and see if I can fix it.

developedby commented 4 years ago

After tinkering around a bit, I reached the conclusion that if ret_cam_errcheck is a RetHandler, then it shouldn't be calling niceobj.GetError(), as that calls a RetHandler to handle the returns, which is itself, thus causing the infinite recursion.

I think the message that should be displayed is Invalid camera handle, as defined on UC480Error, but I don't know how to format the error message if GetError is not called.

My solution was to replace

def ret_cam_errcheck(result, niceobj):
    if result != NiceUC480.SUCCESS:
        err_code, err_msg = niceobj.GetError()
        raise UC480Error(code=result, msg=err_msg)

with

def ret_cam_errcheck(result, niceobj):
    if result != NiceUC480.SUCCESS:
        raise UC480Error(code=result)

But that does not print very pretty or descriptive errors.

I'm also now getting another error, when deleting the object to create a new one.

File "C:\Users\SAO\AppData\Local\Programs\Python\Python37\lib\site-packages\instrumental_lib-0.6.dev0-py3.7.egg\instrumental\drivers\cameras\uc480.py", line 610, in __del__
    if self._in_use:
AttributeError: 'UC480_Camera' object has no attribute '_in_use'

I don't understand why this happens, considering that self._in_use is defined in one of the first lines of UC480_Camera.

I also don't know why I'm getting errors at all.

natezb commented 4 years ago

Yeah, since GetError() is used within the ret_cam_errcheck RetHandler, we it probably shouldn't use that as its own handler. I think explicitly setting its handler to ret_errcheck instead should fix the issue. It's also worth double-checking the signature of IS_GetError to decide what kind of handler makes the most sense.

Also, as far as your _in_use issue goes, it's probably tied to the trickiness of __del__ methods. Perhaps the attribute has already been deleted by the interpreter. At this point, that method can probably be removed altogether since I think the base Instrument class handles auto-closing instruments now. The uc480 driver was one of the first ones, so it may have some out-of-date code still.

developedby commented 4 years ago

I don't really understand how NiceLib works, so I'm not sure how to explicitly setting its handler to ret_errcheck instead.

Removing __del__ does not break anything, as you mentioned.

But still, I should probably be able to open a camera, close it, and then open it again, which is impossible right now.

natezb commented 4 years ago

I just pushed a change that should fix the issue with the error messages.

As for your other issue, can you provide a minimal code sample that produces the error, so I can understand your use-case and maybe the origins of the error?

developedby commented 4 years ago
from instrumental.drivers.cameras import uc480
instruments = uc480.list_instruments()
cam = uc480.UC480_Camera(instruments[0])
cam.grab_image()  # This works
print(cam._in_use)  # True
cam.close()
cam = uc480.UC480_Camera(instruments[0])
print(cam._in_use)  # False
cam.grab_image()  # This fails with the new error message

All the other camera functions fail as well, including cam.close().

natezb commented 4 years ago

Could you run the following and provide the full output you get (logging and the error traceback)? I no longer have access to these cameras so debugging is more challenging.

from instrumental.log import log_to_screen
log_to_screen()

from instrumental.drivers.cameras import uc480
instruments = uc480.list_instruments()
cam = uc480.UC480_Camera(instruments[0])
cam.grab_image()  # This works
print(cam._in_use)  # True
cam.close()
cam = uc480.UC480_Camera(instruments[0])
print(cam._in_use)  # False
cam.grab_image()  # This fails with the new error message

I think in the end we should probably just eliminate the __del__ method since almost no other instrument uses it to invoke close().

developedby commented 4 years ago

Here's the output

I probably should've been more explicit, but I already deleted __del__, which doesn't make a difference aside from not throwing an ignored exception when deleting the object (the log outputs are the same besides the Exception ignored in: <function UC480_Camera.__del__ at 0x000001E4730E4EE8>).

natezb commented 4 years ago

A couple of things I see in the log:

  1. The error message fix I added isn't working well, the code just covers up the recursion depth exception. I will have to set the RetHandler of GetError.
  2. It looks like the new object isn't getting properly initialized. I think this has to do with the "reuse" reopen policy, but still not 100% clear on the details.

I'll try to take a closer look tonight.

natezb commented 4 years ago

I just pushed a couple of changes: one to hopefully fix the GetError issue, and another to add more logging. Please run this code (it's the same, but with DEBUG log level) and post the log. As of now, it's still not clear to me what's going on.

from instrumental.log import log_to_screen, DEBUG
log_to_screen(level=DEBUG)

from instrumental.drivers.cameras import uc480
instruments = uc480.list_instruments()
cam = uc480.UC480_Camera(instruments[0])
cam.grab_image()  # This works
print(cam._in_use)  # True
cam.close()
cam = uc480.UC480_Camera(instruments[0])
print(cam._in_use)  # False
cam.grab_image()  # This fails with the new error message
developedby commented 4 years ago

Here it is

natezb commented 4 years ago

Ok thanks. Apparently __del__ is getting called on the instrument even though it's being reused. I'm able to reproduce this on my end with a simple test class, so I'll try to debug further after work tonight.

natezb commented 4 years ago

Ok, I finally understand what has been going on, and pushed some changes to clear things up.

First, the __del__ I mentioned was a bit of a red herring. It wasn't being invoked on the instrument in question, but was rather an artifact of the way Instrument._create was implemented. That has now been cleaned up, so errors from __del__ should be gone.

The real issue is this: the default reopen_policy was 'reuse', so the camera returned from your second instantiation was actually still the first. Since _initialize is never called on reused instruments, the camera never called _open() to reopen the connection, leading to the failed grab_image(). I've made a few changes in response to this:

  1. Since the 'reuse' behavior was non-obvious, I've changed the default reopen_policy to 'strict', which will raise an informative error. 'reuse' is still available, but must be requested explicitly.
  2. I've added info in the docs about reopen_policy
  3. I've added a public open() method and is_open attribute to UC480_Camera. Previously, a camera could not be re-opened after closing. (Re-opening needs to be tested, I'm not 100% sure it will work as-is).

Give the changes a try and let me know what you think.