Opened 9 years ago

Closed 9 years ago

#12159 closed enhancement (fixed)

Placing triangulation and normal cones

Reported by: vbraun Owned by: mhampton
Priority: major Milestone: sage-5.0
Component: geometry Keywords:
Cc: novoselt Merged in: sage-5.0.beta9
Authors: Volker Braun Reviewers: Marshall Hampton
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by novoselt)

This ticket implements some more functionality for triangulations:

  • the placing triangulation is a triangulation that can be computed faster than the currently used lexicographic triangulation.
  • the normal cone of a triangulation

Apply

Attachments (4)

trac_12159_separate_triangulation_file.patch (43.9 KB) - added by vbraun 9 years ago.
Initial patch
trac_12159_placing_triangulation.patch (39.0 KB) - added by vbraun 9 years ago.
Initial patch
trac_12159_normal_cone.2.patch (15.4 KB) - added by vbraun 9 years ago.
Updated patch
trac_12159_normal_cone.patch (15.4 KB) - added by vbraun 9 years ago.
Updated patch

Download all attachments as: .zip

Change History (13)

Changed 9 years ago by vbraun

Initial patch

Changed 9 years ago by vbraun

Initial patch

comment:1 Changed 9 years ago by vbraun

  • Description modified (diff)
  • Status changed from new to needs_review

comment:2 Changed 9 years ago by novoselt

  • Description modified (diff)

comment:3 Changed 9 years ago by mhampton

A bunch of doctests fail after installing the TOPCOM package, presumably just because of different choices of triangulations. I can't think of a nice way to deal with that offhand, it would be pretty clunky to force every example to use the Sage-native code.

comment:4 Changed 9 years ago by mhampton

OK, maybe its not so bad. Adding in a few more "set_engine('internal')" and "engine='internal'" lines to element.py in geometry/triangulation is enough to fix all the doctest errors.

comment:5 Changed 9 years ago by mhampton

I imagine that is trivial for Volker to do himself but in case its helpful here is my version that passes the doctests after TOPCOM is installed: element.py

Changed 9 years ago by vbraun

Updated patch

Changed 9 years ago by vbraun

Updated patch

comment:6 Changed 9 years ago by vbraun

Updated patch fixes doctests with TOPCOM installed.

comment:7 Changed 9 years ago by davidloeffler

Apply trac_12159_separate_triangulation_file.patch, trac_12159_placing_triangulation.patch, trac_12159_normal_cone.patch

(for the patchbot)

comment:8 Changed 9 years ago by mhampton

  • Cc novoselt added
  • Reviewers set to Marshall Hampton
  • Status changed from needs_review to positive_review

I have not checked the mathematics of this patch in depth, but it passes all doctests and coverage checks, the documentation looks good, and what I have checked seems correct. The problems with doctests after TOPCOM is installed have been fixed. So I am happy to give it a positive review.

comment:9 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.0.beta9
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.