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

Characteristic polynomial of central Hyperplane arrangement returns wrong result? #30749

Closed
jplab opened this issue Oct 9, 2020 · 22 comments
Closed

Comments

@jplab
Copy link
Contributor

jplab commented Oct 9, 2020

A central hyperplane arrangement must have an even number of regions by central symmetry... yet the one below gets 31 regions(!).

R.<y> = QQ[]
v1 = AA.polynomial_root(AA.common_polynomial(y^2 - 3), RIF(RR(1.7320508075688772), RR(1.7320508075688774)))
v2 = QQbar.polynomial_root(AA.common_polynomial(y^4 - y^2 + 1), CIF(RIF(RR(0.8660254037844386), RR(0.86602540378443871)), RIF(-RR(0.50000000000000011), -RR(0.49999999999999994))))
my_vectors = (vector(AA, [-v1, -1, 1]), vector(AA, [0, 2, 1]), vector(AA,[v1, -1, 1]), vector(AA, [1, 0, 0]), vector(AA, [1/2, AA(-1/2*v2^3 + v2),0]), vector(AA, [-1/2, AA(-1/2*v2^3 + v2), 0]))


sage: H = HyperplaneArrangements(AA,names='xyz')
sage: x,y,z = H.gens()
sage: A = H(backend="normaliz")
sage: for v in my_vectors:
....:     a,b,c = v
....:     A = A.add_hyperplane(a*x + b*y + c*z)
sage: A
Arrangement of 6 hyperplanes of dimension 3 and rank 3
sage: A.n_regions()
31
sage: A.is_central()
True

Here is another failure in characteristic polynomial:

sage: tau = AA((1+sqrt(5))/2)                                                                                                                                                                                      
sage: ncn = [[2*tau+1,2*tau,tau],[2*tau+2,2*tau+1,tau+1],[1,1,1],[tau+1,tau+1,tau],[2*tau,2*tau,tau],[tau+1,tau+1,1],[1,1,0],[0,1,0],[1,0,0],[tau+1,tau,tau]]                                                      
sage: H = HyperplaneArrangements(AA,names='xyz')                                                                                                                                                                   
sage: x,y,z = H.gens()                                                                                                                                                                                             
sage: A = H()                                                                                                                                                                                                      
sage: for v in ncn: 
....:     a,b,c = v 
....:     A = A.add_hyperplane(a*x + b*y + c*z) 
....:                                                                                                                                                                                                              
sage: A.n_regions()
Traceback (most recent call last):
...
ValueError: arrangement cannot simultaneously have h and -h as hyperplane

#30078 fixes this and we add another doctest here.

CC: @kliem @LaisRast @sagetrac-nailuj

Component: geometry

Keywords: hyperplane arrangements, regions

Author: Jonathan Kliem

Branch/Commit: d74929d

Reviewer: Travis Scrimshaw

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

@jplab jplab added this to the sage-9.3 milestone Oct 9, 2020
@fchapoton

This comment has been minimized.

@fchapoton
Copy link
Contributor

comment:2

The char poly gives the same answer

sage: p = A.characteristic_polynomial() : p
x^3 - 6*x^2 + 13*x - 11
sage: p(-1)                                                                     
-31

@jplab
Copy link
Contributor Author

jplab commented Oct 9, 2020

comment:3

Replying to @fchapoton:

The char poly gives the same answer

sage: p = A.characteristic_polynomial() : p
x^3 - 6*x^2 + 13*x - 11
sage: p(-1)                                                                     
-31

Yes, this is how the number of regions is computed up to sign. The hyperplane arrangement seems to be sane: computing .regions() computes the expected (number of) regions.

I asked the number of regions to double-check something and then got this scary answer...

@jplab jplab changed the title Characteristic polynomial of central Hyperplane arrangement results wrong result? Characteristic polynomial of central Hyperplane arrangement returns wrong result? Oct 9, 2020
@sagetrac-nailuj
Copy link
Mannequin

sagetrac-nailuj mannequin commented Oct 9, 2020

comment:5

I encountered this problem in some example a long while ago, before I was contributing to Sage myself. A simple workaround is to have n_regions output len(self.regions()), which was good enough for me. Of course, this is much slower if one doesn’t want to compute the regions anyway. I don’t understand the characteristic polynomial well enough to figure out the error in there.

@jplab

This comment has been minimized.

@mkoeppe
Copy link
Contributor

mkoeppe commented Feb 13, 2021

comment:7

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Feb 13, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Aug 9, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 18, 2021
@sagetrac-nailuj
Copy link
Mannequin

sagetrac-nailuj mannequin commented Mar 16, 2022

comment:10

Replying to @sagetrac-nailuj:

I don’t understand the characteristic polynomial well enough to figure out the error in there.

The characteristic polynomial is obtained through recursive deletions and contractions of hyperplane arrangements. During this process, it may occur that some hyperplane h is included in an arrangement multiple times with different scalings of the defining linear expression which are not properly detected as duplicates. This is due to a defect in the method hyperplane.primitive() addressed in #30078.

@mkoeppe
Copy link
Contributor

mkoeppe commented Mar 21, 2022

Dependencies: #30078

@mkoeppe
Copy link
Contributor

mkoeppe commented Apr 3, 2022

comment:12

Is this defect solved by #30078?

@tscrim
Copy link
Collaborator

tscrim commented Apr 4, 2022

comment:13

It seems like it is. Using the first example, I get

sage: A.n_regions()
24
sage: len(A.regions())
24
sage: A.characteristic_polynomial()(-1)
-24

So it is now consistent. We probably just want to add a doctest for this ticket.

@kliem
Copy link
Contributor

kliem commented Apr 5, 2022

comment:14

#30078 already adds one doctest. So we can either close it as duplicate or add another doctest. Either way is fine with me.


New commits:

1492ed1add another doctest for 30749

@kliem
Copy link
Contributor

kliem commented Apr 5, 2022

Branch: public/30749

@kliem
Copy link
Contributor

kliem commented Apr 5, 2022

Author: Jonathan Kliem

@kliem
Copy link
Contributor

kliem commented Apr 5, 2022

Commit: 1492ed1

@kliem

This comment has been minimized.

@kliem
Copy link
Contributor

kliem commented Apr 5, 2022

Changed dependencies from #30078 to none

@sheerluck
Copy link
Contributor

comment:15
--- a/sage/geometry/hyperplane_arrangement/arrangement.py
+++ b/sage/geometry/hyperplane_arrangement/arrangement.py
@@ -1179,10 +1179,10 @@
             sage: H = HyperplaneArrangements(AA, names='xyz')
             sage: x,y,z = H.gens()
             sage: A = H(backend="normaliz")  # optional - pynormaliz
-            sage: for v in my_vector:        # optional - pyrormaliz
+            sage: for v in my_vectors:       # optional - pynormaliz
             ....:     a, b, c = v
             ....:     A = A.add_hyperplane(a*x + b*y + c*z)
-            sage: A.n_regions()              # optional - pyrormaliz
+            sage: A.n_regions()              # optional - pynormaliz
             32
         """
         if self.base_ring().characteristic() != 0:

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 5, 2022

Changed commit from 1492ed1 to d74929d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 5, 2022

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

d74929dtypos

@tscrim
Copy link
Collaborator

tscrim commented Apr 6, 2022

comment:17

Thank you. LGTM.

@sheerluck, please add your (real) name to the reviewers (if you want).

@tscrim
Copy link
Collaborator

tscrim commented Apr 6, 2022

Reviewer: Travis Scrimshaw

@vbraun
Copy link
Member

vbraun commented Apr 10, 2022

Changed branch from public/30749 to d74929d

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

7 participants