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

Implement Orlik-Solomon algebra of an arrangement #18133

Closed
kcrisman opened this issue Apr 7, 2015 · 57 comments
Closed

Implement Orlik-Solomon algebra of an arrangement #18133

kcrisman opened this issue Apr 7, 2015 · 57 comments

Comments

@kcrisman
Copy link
Member

kcrisman commented Apr 7, 2015

The Orlik-Solomon algebra is a fundamental invariant for an arrangement, which also enables computing scads of useful information about e.g. the complement. But it's purely algebraic. So we should have it (see also #17506).

CC: @tscrim @sagetrac-Stefan @sagetrac-yomcat @chaoxu

Component: geometry

Keywords: hyperplane arrangements

Author: Travis Scrimshaw, William Slofstra

Branch/Commit: 48e46b6

Reviewer: Darij Grinberg

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

@kcrisman kcrisman added this to the sage-6.6 milestone Apr 7, 2015
@dimpase
Copy link
Member

dimpase commented Apr 10, 2015

comment:2

how about algebras generated by reciprocals of linear forms of hyperplanes of the arrangement (cf e.g. http://arxiv.org/abs/math/0105095) ?

@kcrisman
Copy link
Member Author

comment:3

Sure, anything you want! I'm not actually implementing this ticket :-) Basically anything in Orlik/Terao is great, so much of it is computational - I was kind of hoping the matroid folks would see this too. Actually, I thought some of this stuff was going to be in a 'next-gen' book by Alex, Dan and others but all I could find was a draft at Hal Schenck's website. It would be great to get Sage "officially" in that book in the same way it's in the Toric Varieties book he coauthored.

@tscrim
Copy link
Collaborator

tscrim commented Nov 1, 2015

Author: Travis Scrimshaw, William Slofstra

@tscrim
Copy link
Collaborator

tscrim commented Nov 1, 2015

Commit: 90a3fa0

@tscrim
Copy link
Collaborator

tscrim commented Nov 1, 2015

comment:4

Here is an implementation based upon code given to me by William Slofstra.


New commits:

90a3fa0Implementation of Orlik-Solomon algebras.

@tscrim
Copy link
Collaborator

tscrim commented Nov 1, 2015

Branch: public/algebras/orlik_solomon-18133

@tscrim tscrim modified the milestones: sage-6.6, sage-6.10 Nov 1, 2015
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 1, 2015

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

9a305aaDoing a little more caching for speed.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 1, 2015

Changed commit from 90a3fa0 to 9a305aa

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 14, 2015

Changed commit from 9a305aa to b077d31

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Nov 14, 2015

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

2ae367fMerge branch 'public/algebras/orlik_solomon-18133' of trac.sagemath.org:sage into public/algebras/orlik_solomon-18133
b077d31A few little doc tweaks.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 20, 2016

Changed commit from b077d31 to 4997564

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 20, 2016

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

1ee0f5aMerge branch
4997564some doc improvements

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 20, 2016

Changed commit from 4997564 to 2417129

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 20, 2016

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

2417129more

@darijgr
Copy link
Contributor

darijgr commented Jan 20, 2016

comment:10

I didn't know that this algebra could be defined for any matroid! That's really nice.

On the other hand, I'm afraid the code doesn't properly iterate the reduction.

            sage: M4 = matroids.CompleteGraphic(4)
            sage: OS = M4.orlik_solomon_algebra(QQ)
            sage: OS._reduce_broken_circuit(frozenset({2,3,4}))
            OS{1, 3, 4} + OS{1, 2, 3} - OS{1, 2, 4}

The result is malformed: {1, 3, 4} itself contains a broken circuit, so one of the monomials isn't actually a monomial. Now you could argue that this is an underscored method and might do with malformed results if the method calling it knows what it's doing, but first of all I'm not sure whether the CombinatorialFreeModule framework will keep accepting such monomials in the future, and second I cannot guarantee that the multiplication as you implemented it still works with this implementation of _reduce_broken_circuit.

The point is, reducing an element modulo the ideal J(M) is a multistep process, as clearing out one broken circuit might create another. If you do it until no more broken circuits remain, then I think your product is correct (though I'll have to take another look).

Another issue is the algebra_generators: You're setting the i-th generator to be self.monomial(frozenset([i])), but this too might be malformed. (For example, if M is a uniform matroid U_{n,1} = matroids.Uniform(1,n), then all basis elements are equal, which means that they cannot be distinct basis elements.)

I think both of these issues can be fixed at once. Let me do it.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 20, 2016

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

6e2baabproperly implementing straightening rule

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 20, 2016

Changed commit from 2417129 to 6e2baab

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 20, 2016

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

48c8515add an observation used in the algorithm

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 20, 2016

Changed commit from 6e2baab to 48c8515

@darijgr
Copy link
Contributor

darijgr commented Jan 20, 2016

comment:13

OK, the above points are done.

I am out of time now, so if anyone wants to take this over, feel free to do so. If not, I'll probably return to this in a week or so.

Meanwhile, here is one more question: Your implementation of the Orlik-Solomon algebra for a hyperplane arrangement assumes that the base field of the algebra is that of the arrangement. Is that a reasonable assumption? (To me it sounds wrong -- like studying the homology of real manifolds only with real coefficients.)

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 20, 2016

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

a7bcbadpostscript

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 20, 2016

Changed commit from 48c8515 to a7bcbad

@tscrim
Copy link
Collaborator

tscrim commented Jan 20, 2016

comment:15

Replying to @darijgr:

On the other hand, I'm afraid the code doesn't properly iterate the reduction.

            sage: M4 = matroids.CompleteGraphic(4)
            sage: OS = M4.orlik_solomon_algebra(QQ)
            sage: OS._reduce_broken_circuit(frozenset({2,3,4}))
            OS{1, 3, 4} + OS{1, 2, 3} - OS{1, 2, 4}

The result is malformed: {1, 3, 4} itself contains a broken circuit, so one of the monomials isn't actually a monomial. Now you could argue that this is an underscored method and might do with malformed results if the method calling it knows what it's doing,

That was going to be my argument.

but first of all I'm not sure whether the CombinatorialFreeModule framework will keep accepting such monomials in the future,

It should because those checks are expensive and there has to be a way to tell this proposed CFM that you know what you're doing (especially in internal methods like _from_dict).

and second I cannot guarantee that the multiplication as you implemented it still works with this implementation of _reduce_broken_circuit.

The point is, reducing an element modulo the ideal J(M) is a multistep process, as clearing out one broken circuit might create another. If you do it until no more broken circuits remain, then I think your product is correct (though I'll have to take another look).

That is how the product was designed and why it worked. Although now you've basically pushed that into the recursive step. This also has an effect of making it recursive. Have you done some timing benchmarks between the two implementations?

Also this is bad:

if not type(S) == frozenset:

You should use the pythonic (and more robust if we replace it by some subclass of frozenset):

if not isinstance(S, frozenset):

I can change this though.

Another issue is the algebra_generators: You're setting the i-th generator to be self.monomial(frozenset([i])), but this too might be malformed. (For example, if M is a uniform matroid U_{n,1} = matroids.Uniform(1,n), then all basis elements are equal, which means that they cannot be distinct basis elements.)

This is a good point. Good catch.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 21, 2016

Changed commit from 66a3be1 to bd1b8d7

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 21, 2016

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

d736c56Added Orlik-Solomon algebra to the documentation.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 21, 2016

Changed commit from bd1b8d7 to d736c56

@tscrim
Copy link
Collaborator

tscrim commented Jan 21, 2016

Reviewer: Darij Grinberg

@tscrim
Copy link
Collaborator

tscrim commented Jan 21, 2016

comment:27

Whoops, sorry; bad merge. You can safely ignore comment:24.

I fixed any issues with comparisons using frozenset by implementing a term comparison function. I also added a few more doctests. Thanks for changing the behavior for creating the Orlik-Solomon. I'm happy with the current version. If you are too (and there are no other objectios), then we can set a positive review.

@tscrim tscrim modified the milestones: sage-6.10, sage-7.1 Jan 21, 2016
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 22, 2016

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

cdc471amore doctests

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 22, 2016

Changed commit from d736c56 to cdc471a

@darijgr
Copy link
Contributor

darijgr commented Jan 22, 2016

comment:29

Thanks! This fixes it.

The code looks good to me now. Feel free to set it to positive_review, or wait for others to comment, if you feel so inclined.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 22, 2016

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

e51cb6dA last little bit of doc tweaks.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 22, 2016

Changed commit from cdc471a to e51cb6d

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 22, 2016

Changed commit from e51cb6d to ce4a0cb

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 22, 2016

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

ce4a0cbMoving things around slightly.

@tscrim
Copy link
Collaborator

tscrim commented Jan 22, 2016

comment:32

Back to you; if you're happy with my changes, then you can set a positive review. Thanks.

@darijgr
Copy link
Contributor

darijgr commented Jan 22, 2016

comment:33

Perfect!

@vbraun
Copy link
Member

vbraun commented Jan 23, 2016

comment:34
sage -t --long src/sage/matroids/matroid.pyx
**********************************************************************
File "src/sage/matroids/matroid.pyx", line 2912, in sage.matroids.matroid.Matroid.orlik_solomon_algebra
Failed example:
    OS = M.orlik_solomon_algebra(QQ)
Expected:
    Orlik-Solomon algebra of U(3, 4): Matroid of rank 3 on 4 elements
     with circuit-closures
     {3: {{0, 1, 2, 3}}}
Got:
    <BLANKLINE>
**********************************************************************
1 item had failures:
   1 of   3 in sage.matroids.matroid.Matroid.orlik_solomon_algebra
    [775 tests, 1 failure, 2.64 s]

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 23, 2016

Changed commit from ce4a0cb to 48e46b6

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jan 23, 2016

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

48e46b6Fixing trivial doctest failure.

@tscrim
Copy link
Collaborator

tscrim commented Jan 23, 2016

comment:36

Stupid me; trivial doctest fix.

@darijgr
Copy link
Contributor

darijgr commented Jan 23, 2016

comment:37

Oops, that was me being stupid too. The only file I've run the tests on was orlik-solomon.py...

@vbraun
Copy link
Member

vbraun commented Jan 24, 2016

Changed branch from public/algebras/orlik_solomon-18133 to 48e46b6

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

5 participants