pepkit / looper

A job submitter for Portable Encapsulated Projects
http://looper.databio.org
BSD 2-Clause "Simplified" License
20 stars 7 forks source link

RecursionError when running bedmaker with Looper #314

Closed xuebingjie1990 closed 1 year ago

xuebingjie1990 commented 3 years ago

Got RecursionError when running bedmaker with Lopper. Here is the full Traceback:

Looper version: 1.3.0
Command: run
Traceback (most recent call last):
  File "/home/bx2ur/.local/bin/looper", line 10, in <module>
    sys.exit(main())
  File "/home/bx2ur/.local/lib/python3.7/site-packages/looper/looper.py", line 743, in main
    run(args, rerun=(args.command == "rerun"), **compute_kwargs)
  File "/home/bx2ur/.local/lib/python3.7/site-packages/looper/looper.py", line 308, in __call__
    for schema_file in self.prj.get_schemas(self.prj.pipeline_interfaces)]
  File "/home/bx2ur/.local/lib/python3.7/site-packages/looper/looper.py", line 308, in <listcomp>
    for schema_file in self.prj.get_schemas(self.prj.pipeline_interfaces)]
  File "/home/bx2ur/.local/lib/python3.7/site-packages/eido/eido.py", line 221, in validate_config
    project_dict = project.to_dict()
  File "/home/bx2ur/.local/lib/python3.7/site-packages/attmap/pathex_attmap.py", line 102, in to_dict
    return self._simplify_keyvalue(self.items(expand), dict)
  File "/home/bx2ur/.local/lib/python3.7/site-packages/attmap/_att_map_like.py", line 225, in _simplify_keyvalue
    return self._simplify_keyvalue(kvs, build, acc, conversions)
  File "/home/bx2ur/.local/lib/python3.7/site-packages/attmap/_att_map_like.py", line 225, in _simplify_keyvalue
    return self._simplify_keyvalue(kvs, build, acc, conversions)
  File "/home/bx2ur/.local/lib/python3.7/site-packages/attmap/_att_map_like.py", line 225, in _simplify_keyvalue
    return self._simplify_keyvalue(kvs, build, acc, conversions)
  [Previous line repeated 10 more times]
  File "/home/bx2ur/.local/lib/python3.7/site-packages/attmap/_att_map_like.py", line 218, in _simplify_keyvalue
    v = self._simplify_keyvalue(list(v.items()), build, build())
  File "/home/bx2ur/.local/lib/python3.7/site-packages/attmap/_att_map_like.py", line 225, in _simplify_keyvalue
    return self._simplify_keyvalue(kvs, build, acc, conversions)
  File "/home/bx2ur/.local/lib/python3.7/site-packages/attmap/_att_map_like.py", line 225, in _simplify_keyvalue
    return self._simplify_keyvalue(kvs, build, acc, conversions)
  File "/home/bx2ur/.local/lib/python3.7/site-packages/attmap/_att_map_like.py", line 225, in _simplify_keyvalue
    return self._simplify_keyvalue(kvs, build, acc, conversions)
  [Previous line repeated 968 more times]
  File "/home/bx2ur/.local/lib/python3.7/site-packages/attmap/_att_map_like.py", line 217, in _simplify_keyvalue
    if is_custom_map(v):
  File "/home/bx2ur/.local/lib/python3.7/site-packages/attmap/helpers.py", line 93, in is_custom_map
    return isinstance(obj, Mapping) and type(obj) is not dict
  File "/apps/software/standard/core/anaconda/2019.10-py3.7/lib/python3.7/abc.py", line 139, in __instancecheck__
    return _abc_instancecheck(cls, instance)
RecursionError: maximum recursion depth exceeded in comparison

The PEP used was successfully validated with eido.

stolarczyk commented 3 years ago

alright, I figured it out. It's not a bug, it's exactly what the exception message says: the recursion limit has been reached in _simplify_keyvalue. This PEP consists of over 17k samples. The recursion limit is exceeded and the RecursionError is thrown. If you set sys.setrecursionlimit(20000) the error goes away.

Maybe we could automatically call sys.setrecursionlimit(n) where n is the number of samples in peppy.Project constructor

nsheff commented 3 years ago

wait, why does it need to recurse at all? It's a simple lookup, it should not be needing to call a recursive function...

Also, @xuebingjie1990, it's looper, not lopper :)...

stolarczyk commented 3 years ago

yeah, that's a good point. The issue emerges when looper.Project.to_dict() is called. So unless it has a 1000x nested attribute, the recursion limit should not be reached.

xuebingjie1990 commented 3 years ago

Also, @xuebingjie1990, it's looper, not lopper :)...

Right, sorry about the typo...

xuebingjie1990 commented 3 years ago

If you set sys.setrecursionlimit(20000) the error goes away.

I'll try run with this first.

nsheff commented 3 years ago

Here's where the recursion is happening:

https://github.com/pepkit/attmap/blob/54b943e597023c8dd6eac8e22b9f210ba8e60517/attmap/_att_map_like.py#L205-L228

nsheff commented 3 years ago

@vreuter do you have any insight on why a PEP with lots of rows would trigger thousands of recursive calls to that function in attmap?

Just on the surface it seems like that should require thousands of nested elements...but we're not talking about nesting like that, it's just a table with lots of rows, so I don't see why it should be triggering recursion. thought you might have some insight since you wrote that function

nsheff commented 1 year ago

I wonder if this would go away if we dropped attmap altogether.

nsheff commented 1 year ago

This issue will be a good check to see if it's resolved with the attmap dependency removed, plus improvements on the peppy side. I'm going to bump to 1.5.0

@xuebingjie1990 can you check this again once looper 1.4.0 is released (hopefully today) ? If it's still causing a problem then we'll revisit for 1.5.0

xuebingjie1990 commented 1 year ago

@xuebingjie1990 can you check this again once looper 1.4.0 is released (hopefully today) ? If it's still causing a problem then we'll revisit for 1.5.0

I was able to submit jobs without the error.

nsheff commented 1 year ago

Hooray! Looks like our performance is much better in the new looper.