search

3-manifolds / SnapPy

SnapPy is a package for studying the topology and geometry of 3-manifolds, with a focus on hyperbolic structures. It is based on the SnapPea kernel written by Jeff Weeks.
https://snappy.computop.org/
84 stars 39 forks source link

Hang in the cusp neighbourhood code #11

Open unhyperbolic opened 4 years ago

unhyperbolic commented 4 years ago

Reported hang by Saul Schleimer:

sage: sig = 'nLLzLMAPAkcdchfghjkjklmmhsrsbwgarbgahr' sage: M = snappy.Manifold(sig) sage: C = M.cusp_neighborhood() sage: C.set_tie(0, 2)

unhyperbolic commented 4 years ago

Nathan also observed the following failure:

M = Manifold('nLLzLMAPAkcdchfghjkjklmmhsrsbwgarbgahr') C = M.cusp_neighborhood() C.set_displacement(C.get_displacement(which_cusp=0), which_cusp=0)

unhyperbolic commented 4 years ago

https://github.com/3-manifolds/SnapPy/commit/5755f76a72849d8060eeb11958adfe04e1928dd2 is avoiding the hang, but ideally, we would make this code fail less.

NathanDunfield commented 4 years ago

is avoiding the hang, but ideally, we would make this code fail less.

It would be nice if we could reproduce the problem outside of the context of cusp_neighborhood. As I understand the traceback, the uFatalError is generated after a call to proto_canonize returns func_failed. But M.canonize(), which simply calls proto_canonize, doesn't give an error with Saul's triangulation. But maybe cusp_neighborhood changes the relevant cusp sizes used to define "canonical" in this setting.

In my example, the error message is somewhat different:

SnapPeaFatalError: SnapPea crashed in function remove_edge_of_order_one(), defined in simplify_triangulation.c.

I guess possibly the fatal error message has been overwritten by later calls to uFatalError. Perhaps to figure this out we need to have uFatalError generate a SEGFAULT so we can poke around at the C level with a debugger.

culler commented 4 years ago

How about just setting a break point in uFatalError? I think that's the standard approach.

On Mon, Jan 20, 2020, 12:48 PM Nathan Dunfield notifications@github.com wrote:

is avoiding the hang, but ideally, we would make this code fail less.

It would be nice if we could reproduce the problem outside of the context of cusp_neighborhood. As I understand the traceback, the uFatalError is generated after a call to proto_canonize returns func_failed. But M.canonize(), which simply calls proto_canonize, doesn't give an error with Saul's triangulation. But maybe cusp_neighborhood changes the relevant cusp sizes used to define "canonical" in this setting.

In my example, the error message is somewhat different:

SnapPeaFatalError: SnapPea crashed in function remove_edge_of_order_one(), defined in simplify_triangulation.c.

I guess possibly the fatal error message has been overwritten by later calls to uFatalError. Perhaps to figure this out we need to have uFatalError generate a SEGFAULT so we can poke around at the C level with a debugger.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/3-manifolds/SnapPy/issues/11?email_source=notifications&email_token=AAJ6CP47BEOVYIIO2USVS73Q6XWV7A5CNFSM4KJHWCA2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJNQ6PY#issuecomment-576393023, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJ6CP7S5JSBKMJZEZYY2PLQ6XWV7ANCNFSM4KJHWCAQ .

unhyperbolic commented 4 years ago

FYI, I checked in a change to uFatalError a couple of weeks ago so that a later error no longer overwrites the first error. As for getting stack traces: it saddens me that dumping a stack trace is not standard in libc or libc++ and requires some platform dependent code (see, e.g., https://github.com/PixarAnimationStudios/USD/blob/master/pxr/base/lib/arch/stackTrace.cpp#L1311). I would love for uFatalError to raise a python exception that includes a stack trace either as extra data or folded into the traceback object associated with the python exception.

Another thought: I wonder whether we should change the signature of M.canonize to possibly include a list of cusp areas.