Fix Polyhedron.volume() in 0-dimensional space #27673

kliem · 2019-04-15T20:30:42Z

In dimension 0, we now use the counting measure for volume:

sage: Polyhedron(vertices=[[]]).volume()
1
sage: Polyhedron(vertices=[]).volume()
0

Originally there was an attribute error:

sage: Polyhedron(vertices=[[]]).volume()
Traceback (most recent call last)
...
AttributeError: 'int' object has no attribute 'set_immutable'

CC: @jplab

Component: geometry

Keywords: polytopes, volume

Author: Jonathan Kliem

Branch/Commit: a7bf147

Reviewer: Jean-Philippe Labbé

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

The text was updated successfully, but these errors were encountered:

kliem · 2019-04-15T21:51:37Z

Branch: public/27673

kliem · 2019-04-15T21:51:37Z

New commits:

`3bd7a03`	`fixed AttributeError of Polyhedron.volume()`

kliem · 2019-04-15T21:51:37Z

Commit: 3bd7a03

kliem · 2019-04-16T07:18:13Z

comment:4

When thinking about it again, I'm not sure anymore that this is the correct output.

However, an AttributeError is strange and the dostring does not indicate, what should happen.

Also it's confusing that the empty Polyhedron gives an output.

jplab · 2019-04-16T07:42:12Z

comment:5

This might be related to #17339. If you want to have fun in the maze of defaulting behaviour, I challenge you to come up with a full-proof default behavior that will please everyone.

One thing that you should be aware of, is the current handling of things via different H and V inputs.

sage: P = Polyhedron(vertices=[])
sage: Q = Polyhedron(vertices=[[]])
sage: P
The empty polyhedron in ZZ^0
sage: Q
A 0-dimensional polyhedron in ZZ^0 defined as the convex hull of 1 vertex

The first one is the empty polyhedron. The second one is just one point, i.e. the full space. They are __not__ the same.

kliem · 2019-04-16T08:19:20Z

comment:7

Well, I can live with a meaningful 'NotImplementedError'.

Either there is a measure with ambient dimension zero or not. So the empty and the one point polyhedron in zero dimensional space should either have a well-defined behaiviour or not.

The reason I looked into it is that the original error is hard to trace back. It took me a bit to figure out, what went wrong there.

Replying to @jplab:

This might be related to #17339. If you want to have fun in the maze of defaulting behaviour, I challenge you to come up with a full-proof default behavior that will please everyone.

One thing that you should be aware of, is the current handling of things via different H and V inputs.
sage: P = Polyhedron(vertices=[])
sage: Q = Polyhedron(vertices=[[]])
sage: P
The empty polyhedron in ZZ^0
sage: Q
A 0-dimensional polyhedron in ZZ^0 defined as the convex hull of 1 vertex
The first one is the empty polyhedron. The second one is just one point, i.e. the full space. They are __not__ the same.

sagetrac-git · 2019-04-16T13:18:20Z

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

`cfa12c7`	`volume in dimension 0 is counting measure`

sagetrac-git · 2019-04-16T13:18:20Z

Changed commit from 3bd7a03 to cfa12c7

sagetrac-git · 2019-04-16T13:31:49Z

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

`7995b24`	`a bit nicer documentation`

sagetrac-git · 2019-04-16T13:31:49Z

Changed commit from cfa12c7 to 7995b24

jplab · 2019-04-26T12:28:02Z

comment:11

You should change the title and description of the ticket to reflect faithfully the changes inside the ticket.

jplab · 2019-04-26T12:29:43Z

comment:12

Failed doctest:

sage -t --long src/sage/geometry/polyhedron/base.py
**********************************************************************
File "src/sage/geometry/polyhedron/base.py", line 5251, in sage.geometry.polyhedron.base.Polyhedron_base.volume
Failed example:
    P = Polyhedron(vertices=[]); P
Expected:
    The empty polyhedron in ZZ^0
    0
Got:
    The empty polyhedron in ZZ^0
**********************************************************************

sagetrac-git · 2019-05-07T19:22:45Z

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

`dd6f981`	`Merge branch 'develop' of git://trac.sagemath.org/sage into public/27673`
`98e05b5`	`fixed doctest mistake`

sagetrac-git · 2019-05-07T19:22:45Z

Changed commit from 7995b24 to 98e05b5

jplab · 2019-05-10T08:44:27Z

comment:16

Looks good to me now.

jplab · 2019-05-10T08:44:27Z

Changed keywords from none to polytopes, volume

vbraun · 2019-05-11T15:06:46Z

comment:17

Merge conflict, but isn't it also also mathematically wrong:

In the "ambient" measure, if ambient dim > 0, the volume of a point is 0
In the "ambient" measure, if ambient dim == 0, the volume of a point is 1
In the induced measures, the volume of a point is 1
The volume of the empty polyhedron is always 0

kliem · 2019-05-11T16:26:48Z

comment:18

Replying to @vbraun:

Merge conflict, but isn't it also also mathematically wrong:

In the "ambient" measure, if ambient dim > 0, the volume of a point is 0

See line above. Which returns 0 in that case.

In the "ambient" measure, if ambient dim == 0, the volume of a point is 1

In the induced measures, the volume of a point is 1

A point is projected to 0-dimensional space an then 1 is returned.

The volume of the empty polyhedron is always 0

It has dimension -1 but ambient dimension at least 0, so zero will be returned.

I don't see which criteria is not met.

vbraun · 2019-05-11T22:59:50Z

comment:19

Oh ok, I misread the docstring.

Still a merge conflict though.

sagetrac-git · 2019-05-12T18:04:40Z

Branch pushed to git repo; I updated commit sha1. This was a forced push. Last 10 new commits:

`b543c6d`	`remove an obsolete note in schemes, pep cleanup of zariski_vankampen`
`12373c6`	`Change format of a 'not tested' doc test annotation`
`8783bec`	`leave GMP's spkg-configure.m4 as it was`
`64f4b4d`	`update docs, as discussed on #24905`
`2fa1373`	`Mention libterm-readkey-perl`
`406ece2`	`Mention libterm-readline-gnu-perl`
`a031f09`	`polymake: Mention MOngoDB for polydb`
`473178e`	`polymake: Update info on macOS`
`9a4bac1`	`polymake: update MongoDB information for linux distros`
`343130c`	`Updated [SageMath](../wiki/SageMath) version to 8.8.beta5`

sagetrac-git · 2019-05-12T18:04:40Z

Changed commit from 98e05b5 to 343130c

sagetrac-git · 2019-05-12T18:43:37Z

Changed commit from 343130c to 33b1ff9

sagetrac-git · 2019-05-12T18:43:37Z

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

`33b1ff9`	`fixed 0-dimensional volume`

kliem · 2019-05-28T13:29:32Z

comment:23

Why does the merge fail again?

Last time I fixed it, was appearently a waste of time.
I guess I could start a new branch again, but only if someone sets it to positive review before the merge fails again.

jplab · 2019-05-29T08:52:10Z

comment:24

Replying to @kliem:

Why does the merge fail again?

Last time I fixed it, was appearently a waste of time.
I guess I could start a new branch again, but only if someone sets it to positive review before the merge fails again.

#25091 Modified the volume function and got merged in 8.8.beta6 that's most likely the conflict.

sagetrac-git · 2019-05-31T21:14:24Z

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

`7a7e57d`	`volume in dimension 0 is counting measure`

sagetrac-git · 2019-05-31T21:14:24Z

Changed commit from 33b1ff9 to 7a7e57d

sagetrac-git · 2019-05-31T21:19:30Z

Changed commit from 7a7e57d to df1f549

sagetrac-git · 2019-05-31T21:19:30Z

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

`df1f549`	`should be back to the original`

sagetrac-git · 2019-05-31T21:23:12Z

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

`a7bf147`	`fixed 0-dimensional volume`

sagetrac-git · 2019-05-31T21:23:12Z

Changed commit from df1f549 to a7bf147

kliem · 2019-05-31T21:24:15Z

comment:29

I have no idea, how one is supposed to get rebase to work.

This is basically just writing the old text in the newest develop again.

jplab · 2019-06-01T09:22:40Z

comment:30

Looks good to me now.

vbraun · 2019-06-05T18:31:07Z

Changed branch from public/27673 to a7bf147

kliem added this to the sage-8.8 milestone Apr 15, 2019

kliem added c: geometry labels Apr 15, 2019

This comment has been minimized.

Sign in to view

kliem added the s: needs review label Apr 15, 2019

This comment has been minimized.

Sign in to view

jplab added s: needs work and removed s: needs review labels Apr 26, 2019

This comment has been minimized.

Sign in to view

kliem changed the title ~~AttributeError of Polyhedron.volume()~~ Fix Polyhedron.volume() in 0-dimensional space May 7, 2019

kliem added s: needs review and removed s: needs work labels May 7, 2019

vbraun added s: needs work and removed s: positive review labels May 11, 2019

kliem added s: needs review and removed s: needs work labels May 12, 2019

jplab added s: needs work and removed s: needs review labels May 29, 2019

kliem added s: needs review and removed s: needs work labels May 31, 2019

jplab added s: positive review and removed s: needs review labels Jun 1, 2019

vbraun removed the s: positive review label Jun 5, 2019

vbraun closed this as completed in 23c6588 Jun 5, 2019

Fix Polyhedron.volume() in 0-dimensional space #27673

Fix Polyhedron.volume() in 0-dimensional space #27673

Comments

kliem commented Apr 15, 2019

This comment has been minimized.

kliem commented Apr 15, 2019

kliem commented Apr 15, 2019

kliem commented Apr 15, 2019

kliem commented Apr 16, 2019

This comment has been minimized.

jplab commented Apr 16, 2019

kliem commented Apr 16, 2019

sagetrac-git mannequin commented Apr 16, 2019

sagetrac-git mannequin commented Apr 16, 2019

This comment has been minimized.

sagetrac-git mannequin commented Apr 16, 2019

sagetrac-git mannequin commented Apr 16, 2019

jplab commented Apr 26, 2019

jplab commented Apr 26, 2019

This comment has been minimized.

sagetrac-git mannequin commented May 7, 2019

sagetrac-git mannequin commented May 7, 2019

jplab commented May 10, 2019

jplab commented May 10, 2019

vbraun commented May 11, 2019

kliem commented May 11, 2019

vbraun commented May 11, 2019

sagetrac-git mannequin commented May 12, 2019

sagetrac-git mannequin commented May 12, 2019

sagetrac-git mannequin commented May 12, 2019

sagetrac-git mannequin commented May 12, 2019

kliem commented May 28, 2019

jplab commented May 29, 2019

sagetrac-git mannequin commented May 31, 2019

sagetrac-git mannequin commented May 31, 2019

sagetrac-git mannequin commented May 31, 2019

sagetrac-git mannequin commented May 31, 2019

sagetrac-git mannequin commented May 31, 2019

sagetrac-git mannequin commented May 31, 2019

kliem commented May 31, 2019

jplab commented Jun 1, 2019

vbraun commented Jun 5, 2019