Opened 9 months ago

Closed 9 months ago

#30204 closed enhancement (fixed)

Prepare Polyhedra parent factory to handle more general ambient spaces

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.2
Component: geometry Keywords:
Cc: gh-kliem, jipilab Merged in:
Authors: Matthias Koeppe Reviewers: Jonathan Kliem
Report Upstream: N/A Work issues:
Branch: ab7c5ff (Commits, GitHub, GitLab) Commit: ab7c5ff933880df5fd645fcbc82acb25b667fe88
Dependencies: Stopgaps:

Status badges

Description (last modified by mkoeppe)

This adds a new way to set up a Polyhedra parent.

        sage: V = VectorSpace(QQ, 3)
        sage: Polyhedra(V) is Polyhedra(QQ, 3)
        True

Part of #30198.

Change History (9)

comment:1 Changed 9 months ago by mkoeppe

  • Branch set to u/mkoeppe/generalize_polyhedra_parent_factory_to_handle_more_general_ambient_spaces

comment:2 Changed 9 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Commit set to 20082d5132efccf048686b596b24700465dd2d57
  • Description modified (diff)
  • Status changed from new to needs_review
  • Summary changed from Generalize Polyhedra parent factory to handle more general ambient spaces to Prepare Polyhedra parent factory to handle more general ambient spaces

New commits:

20082d5sage.geometry.polyhedron.parent.Polyhedra: Generalize the factory

comment:3 Changed 9 months ago by gh-kliem

sage: V = FiniteRankFreeModule(QQ, 2)
sage: Polyhedra(V)
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
/home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/structure/category_object.pyx in sage.structure.category_object.CategoryObject.getattr_from_category (build/cythonized/sage/structure/category_object.c:6954)()
    837         try:
--> 838             return self.__cached_methods[name]
    839         except KeyError:

KeyError: 'dimension'

During handling of the above exception, another exception occurred:

AttributeError                            Traceback (most recent call last)
<ipython-input-30-0c140d68c4dd> in <module>()
----> 1 Polyhedra(V)

/home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/geometry/polyhedron/parent.py in Polyhedra(ambient_space_or_base_ring, ambient_dim, backend, ambient_space, base_ring)
    113             base_ring = ambient_space.base_ring()
    114         if ambient_dim is None:
--> 115             ambient_dim = ambient_space.dimension()
    116         if ambient_space is not FreeModule(base_ring, ambient_dim):
    117             raise NotImplementedError('ambient_space must be standard free module')

/home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/structure/category_object.pyx in sage.structure.category_object.CategoryObject.__getattr__ (build/cythonized/sage/structure/category_object.c:6876)()
    830             AttributeError: 'PrimeNumbers_with_category' object has no attribute 'sadfasdf'
    831         """
--> 832         return self.getattr_from_category(name)
    833 
    834     cdef getattr_from_category(self, name):

/home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/structure/category_object.pyx in sage.structure.category_object.CategoryObject.getattr_from_category (build/cythonized/sage/structure/category_object.c:7039)()
    845                 cls = self._category.parent_class
    846 
--> 847             attr = getattr_from_other_class(self, cls, name)
    848             self.__cached_methods[name] = attr
    849             return attr

/home/jonathan/Applications/sage/local/lib/python3.7/site-packages/sage/cpython/getattr.pyx in sage.cpython.getattr.getattr_from_other_class (build/cythonized/sage/cpython/getattr.c:2548)()
    387         dummy_error_message.cls = type(self)
    388         dummy_error_message.name = name
--> 389         raise AttributeError(dummy_error_message)
    390     cdef PyObject* attr = instance_getattr(cls, name)
    391     if attr is NULL:

AttributeError: 'FiniteRankFreeModule_with_category' object has no attribute 'dimension'

I think we should add dimension as an alias for rank in FiniteRankFreeModule.

comment:4 follow-up: Changed 9 months ago by mkoeppe

Thanks for catching this. I'm working on an improvement and will add some more tests as well.

Let's not do changes to the modules API on this ticket.

comment:5 in reply to: ↑ 4 Changed 9 months ago by mkoeppe

Replying to mkoeppe:

Let's not do changes to the modules API on this ticket.

I've created #30215 for dimension.

comment:6 Changed 9 months ago by git

  • Commit changed from 20082d5132efccf048686b596b24700465dd2d57 to ab7c5ff933880df5fd645fcbc82acb25b667fe88

Branch pushed to git repo; I updated commit sha1. New commits:

ab7c5ffPolyhedra: Handle ambient_space = a free module correctly

comment:7 Changed 9 months ago by gh-kliem

  • Reviewers set to Jonathan Kliem

For CombinatorialFreeModule and FiniteRankFreeModule I'm getting both NotImplementedError: ambient_space must be a standard free module.

I assume that #30094 takes care of this?

If this is the case, you can put this on positive review on my behalf.

comment:8 Changed 9 months ago by mkoeppe

  • Status changed from needs_review to positive_review

That's right. Based on #30094 and the present ticket, the functionality will be implemented in #30198.

Thanks!

comment:9 Changed 9 months ago by vbraun

  • Branch changed from u/mkoeppe/generalize_polyhedra_parent_factory_to_handle_more_general_ambient_spaces to ab7c5ff933880df5fd645fcbc82acb25b667fe88
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.