Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prepare Polyhedra parent factory to handle more general ambient spaces #30204

Closed
mkoeppe opened this issue Jul 22, 2020 · 14 comments
Closed

Prepare Polyhedra parent factory to handle more general ambient spaces #30204

mkoeppe opened this issue Jul 22, 2020 · 14 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented Jul 22, 2020

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.

CC: @kliem @jplab

Component: geometry

Author: Matthias Koeppe

Branch/Commit: ab7c5ff

Reviewer: Jonathan Kliem

Issue created by migration from https://trac.sagemath.org/ticket/30204

@mkoeppe mkoeppe added this to the sage-9.2 milestone Jul 22, 2020
@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 23, 2020

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 23, 2020

New commits:

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

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 23, 2020

Author: Matthias Koeppe

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 23, 2020

Commit: 20082d5

@mkoeppe mkoeppe changed the title Generalize Polyhedra parent factory to handle more general ambient spaces Prepare Polyhedra parent factory to handle more general ambient spaces Jul 23, 2020
@kliem
Copy link
Contributor

kliem commented Jul 24, 2020

comment:3
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.

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 24, 2020

comment:4

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.

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 24, 2020

comment:5

Replying to @mkoeppe:

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

I've created #30215 for dimension.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 24, 2020

Changed commit from 20082d5 to ab7c5ff

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 24, 2020

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

ab7c5ffPolyhedra: Handle ambient_space = a free module correctly

@kliem
Copy link
Contributor

kliem commented Jul 25, 2020

Reviewer: Jonathan Kliem

@kliem
Copy link
Contributor

kliem commented Jul 25, 2020

comment:7

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.

@mkoeppe
Copy link
Member Author

mkoeppe commented Jul 25, 2020

comment:8

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

Thanks!

@vbraun
Copy link
Member

vbraun commented Jul 28, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants