bradenmacdonald / astrodendro

Deprecated. Python package for computation, visualization, and quantified analysis of astronomical dendrograms
2 stars 0 forks source link

Maximum recursion depth when writing hdf5 #4

Open keflavich opened 12 years ago

keflavich commented 12 years ago

I got the following error after attempting to use dendrogram.to_hdf5:

  File "/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/site-packages/Astronomical_Dendrograms-0.5.0-py2.6.egg/astrodendro/components.py", line 212, in newick
newick_items = [item.newick for item in self.items] 
RuntimeError: maximum recursion depth exceeded

I think I had a very large dendrogram. I attempted to change the maximum recursion level (default is 1000), but that resulted in a segmentation fault. I'm not sure what the best workaround is, but I think at least a try/except testing for a RuntimeError in the newick retriever might be useful.

bradenmacdonald commented 12 years ago

Thanks for the report! I've been away but will look into this and come up with a fix or a more useful workaround when I get a chance.

bradenmacdonald commented 12 years ago

I can confirm the problem with the following testcase:

    def test_deep_tree(self):
        """
        Test writing and reading a dendrogram with many levels to make sure 
        we're not getting recursion errors
        """
        # Construct a deep dendrogram - see test_recursion.py for more comments
        size = 4000 # number of leaves desired in the dendrogram
        data1 = np.arange(size*2) # first row
        data2 = np.arange(size*2) # second row
        data2[::2] += 2;
        d = Dendrogram(np.vstack((data1,data2)), verbose=False)
        # Now, save this to HDF5:
        d.to_hdf5(self.test_filename)
        d2 = Dendrogram()
        d2.from_hdf5(self.test_filename)

As you mentioned, raising the recursion limit does not help, because the Python interpreter will still crash due to resource exhaustion.

It appears a non-recursive newick building is needed. Shouldn't be too hard to implement. The method to be revised is this one.

serenalotreck commented 1 year ago

@bradenmacdonald @keflavich This appears to still be an issue in tethne 0.8, has any progress been made towards a non-recursive solution?

keflavich commented 1 year ago

I'm pretty sure no one has touched this, so at the moment, you're limited to just reducing the size.

serenalotreck commented 1 year ago

@keflavich My corpus only has 25 papers in it, how much more will I need to reduce it?

EDIT: oh wow @keflavich just realized I commented on the completely wrong repository! I'm having the same error in a totally different package that has the same function names, so sorry about that