Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#28155 closed enhancement (fixed)

Check for formality of GCDA's

Reported by: mmarco Owned by:
Priority: major Milestone: sage-9.0
Component: algebraic topology Keywords:
Cc: jhpalmieri, slelievre, dimpase, tscrim Merged in:
Authors: Miguel Marco, Victor Manero Reviewers: Travis Scrimshaw, John Palmieri
Report Upstream: N/A Work issues:
Branch: e95c78a (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by mmarco)

Implement criteria to determine if a graded commutative differential algebra is formal or not.

While I was at it, I also added some auxiliary functions that might be useful, forced it to use a more reasonable ordering when listing the basis of a homogenous part, and cleaned the style a little bit.

The algorithm used is described in https://arxiv.org/abs/1909.07761

Change History (30)

comment:1 Changed 3 years ago by mmarco

  • Branch set to u/mmarco/check_for_formality_of_gcda_s

comment:2 Changed 3 years ago by mmarco

  • Commit set to 77e2d7e0d97223271feab1162dc5ea9f5564719a
  • Description modified (diff)
  • Milestone changed from sage-8.9 to sage-pending

New commits:

3d78783Force ordered degrees, and fix lexicographic order in the basis
cfadaf4Fix indentation error in doctest
4a255b3Merge branch 'develop' of git://github.com/sagemath/sage into formalidad
01de641Implemented numerical invariants and formality
4d3f89cAuxiliar functions for cohomology class, and style fixes
77e2d7eMerge branch 'formalidad' into t/28155/check_for_formality_of_gcda_s

comment:3 Changed 3 years ago by mmarco

  • Component changed from PLEASE CHANGE to algebra
  • Type changed from PLEASE CHANGE to enhancement

comment:4 Changed 3 years ago by git

  • Commit changed from 77e2d7e0d97223271feab1162dc5ea9f5564719a to d16974d85a1e7e8d74b70b2e5ff596dc10dd7d2c

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

d16974dFix criterion for formality

comment:5 Changed 3 years ago by mmarco

  • Description modified (diff)

comment:6 Changed 3 years ago by git

  • Commit changed from d16974d85a1e7e8d74b70b2e5ff596dc10dd7d2c to 87b562c782975d591f48b3b748520878015375c9

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

87b562cAdded some documentation and fixed style

comment:7 Changed 3 years ago by mmarco

  • Authors set to Miguel Marco
  • Cc jhpalmieri slelievre dimpase tscrim added
  • Component changed from algebra to algebraic topology
  • Milestone changed from sage-pending to sage-8.9
  • Status changed from new to needs_review

New commits:

87b562cAdded some documentation and fixed style

comment:8 follow-up: Changed 3 years ago by tscrim

  • Reviewers set to Travis Scrimshaw

It looks good overall. Here are my comments:

You do not need to create the entire list here:

-return sum([a * b for (a, b) in zip(cohomcoefs, C.basis().values())])
+return sum(a * b for (a, b) in zip(cohomcoefs, C.basis().values()))

Also, is that expected to work if C.basis() is empty (if that even occurs)?

Similar here

-if any([N1[n] != N2[n] for n in range(1, i + 1)]):
+if any(N1[n] != N2[n] for n in range(1, i + 1)):

and here

-if all([c.is_coboundary() for c in tocheck]):
+if all(c.is_coboundary() for c in tocheck):

Sage follows python convention of errors starting with lower case:

-raise NotImplementedError("The implemented criteria cannot determine formality")
+raise NotImplementedError("the implemented criteria cannot determine formality")

Our conventions for INPUT: docstrings:

-        - ``i`` -- integer; the degree up to which the formality is checked.
+        - ``i`` -- integer; the degree up to which the formality is checked
 
         - ``max_iterations`` -- integer (default: `3`); the maximum number of
-          iterations used in the computation of the minimal model.
+          iterations used in the computation of the minimal model
         - ``max_degree`` -- integer (default: `3`); the degree up to which the
-          numerical invariants are computed.
+          numerical invariants are computed
 
         - ``max_iterations`` -- integer (default: `3`); the maximum number of iterations
-          used to compute the minimal model, if it is not already cached.
+          used to compute the minimal model, if it is not already cached

It is not always a good idea to strictly follow PEP8 when writing code. It leads to things like this:

                                     ['x{}'.format(i)
                                         for i in range(len(chgens))],

where the break makes it harder to read the code. Not that this will cause me to hold up the ticket once everything else is done, but it might be worthwhile to go through and consider on the changes you made.

comment:9 Changed 3 years ago by git

  • Commit changed from 87b562c782975d591f48b3b748520878015375c9 to f8862305d98785d6760fc010263711d89a1f4d0e

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

f886230Style fixes

comment:10 in reply to: ↑ 8 Changed 3 years ago by mmarco

I made the suggested changes.

Also, is that expected to work if C.basis() is empty (if that even occurs)?

It is expected to return the zero element of a trivial cohomology group. It did return zero as an integer. I have forced a conversion to the corresponding cohomology group, and added a corresponding test.


New commits:

f886230Style fixes

comment:11 Changed 3 years ago by tscrim

Thank you. One last thing to address is the pyflakes (see the patchbot). For instance, since you do not use the minimal model, you can just do this:

-MH = H.minimal_model(i, max_iterations)
+H.minimal_model(i, max_iterations)

comment:12 Changed 3 years ago by git

  • Commit changed from f8862305d98785d6760fc010263711d89a1f4d0e to 0c89c9b493705e11c1fed71f0f3384e429f628ef

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

0c89c9bFixed pyflakes issues

comment:13 Changed 3 years ago by tscrim

  • Status changed from needs_review to positive_review

Thank you.

comment:14 Changed 3 years ago by chapoton

  • Milestone changed from sage-8.9 to sage-9.0

moving milestone to 9.0 (after release of 8.9)

comment:15 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

I'm getting a lot of

File "src/sage/algebras/commutative_dga.py", line 2888, in sage.algebras.commutative_dga.DifferentialGCAlgebra.Element.cohomology_class
Failed example:
    a.cohomology_class()
Expected:
    B[[e1*e3*e5]] - 3*B[[e2*e3*e5]]
Got:
    -3*B[[e2*e3*e5]] + B[[e1*e3*e5]]
**********************************************************************
1 item had failures:
   1 of  11 in sage.algebras.commutative_dga.DifferentialGCAlgebra.Element.cohomology_class
    [600 tests, 1 failure, 26.78 s]
----------------------------------------------------------------------
sage -t --long src/sage/algebras/commutative_dga.py  # 1 doctest failed
----------------------------------------------------------------------

comment:16 Changed 3 years ago by jhpalmieri

Oddly enough, I see this intermittently with Python 2 but not at all with Python 3. In a Python 2 Sage session:

sage: A.<e1,e2,e3,e4,e5> = GradedCommutativeAlgebra(QQ)
sage: B = A.cdg_algebra({e5:e1*e2+e3*e4})
sage: B.inject_variables()
Defining e1, e2, e3, e4, e5
sage: a = e1*e3*e5-3*e2*e3*e5
sage: a.cohomology_class()
B[[e1*e3*e5]] - 3*B[[e2*e3*e5]]
sage: a.cohomology_class()
-3*B[[e2*e3*e5]] + B[[e1*e3*e5]]}}}
Last edited 3 years ago by jhpalmieri (previous) (diff)

comment:17 Changed 3 years ago by git

  • Commit changed from 0c89c9b493705e11c1fed71f0f3384e429f628ef to dc9c7eedf35e6e0f3f5766665518dce5dd153376

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

0253ae3Merge branch 'develop' into t/28155/check_for_formality_of_gcda_s
dc9c7eeMake CohomologyClass CachedRepresentation

comment:18 Changed 3 years ago by mmarco

It seems to be a problem with CohomologyClass. I did it CachedRepresentation, which seems to solve the problem of returning different representations in the same session. However, I still get errors every time I run the testsuite.

Is there any flag or environment variable that could affect the order in which the basis of a CombinatorialFreeModuleare represented depending on the fact that we are in an interactive shell or running the testsuite?


New commits:

0253ae3Merge branch 'develop' into t/28155/check_for_formality_of_gcda_s
dc9c7eeMake CohomologyClass CachedRepresentation

comment:19 Changed 3 years ago by git

  • Commit changed from dc9c7eedf35e6e0f3f5766665518dce5dd153376 to 88f8831fd8a04ce0e64b54e36e67a1f2ed1e6041

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

88f8831Fix order on for representation of cohomology classes

comment:20 Changed 3 years ago by mmarco

  • Status changed from needs_work to needs_review

Ok, I think I solved it.


New commits:

88f8831Fix order on for representation of cohomology classes

comment:21 Changed 3 years ago by jhpalmieri

Using CachedRepresentation should not be necessary, since you're using sorting_key=..., and this seems to be borne out in some testing that I did. So I think it should be removed.

comment:22 Changed 3 years ago by mmarco

I found that ithout CachedRepresentation , each time that the cohomology class of an element is computed, we get a different object. In particular, when we create the cohomology group, it takes as basis the specific instances of the cohomology classes of the generators that were created in that particular call. Hence, when we call it again, we get a different group.

That is, without CachedRepresentation we get this:

sage: A.<e1,e2,e3,e4,e5> = GradedCommutativeAlgebra(QQ)
sage: B = A.cdg_algebra({e5:e1*e2+e3*e4})
sage: B.inject_variables()
Defining e1, e2, e3, e4, e5
sage: a = e1*e3*e5-3*e2*e3*e5
sage: C = B.cohomology(3)
sage: a.cohomology_class() in C
False

Even if the parent of a.cohomology_class() should be C :

sage: C
Free module generated by {[e1*e2*e5 - e3*e4*e5], [e1*e3*e5], [e2*e3*e5], [e1*e4*e5], [e2*e4*e5]} over Rational Field
sage: a.cohomology_class().parent()
Free module generated by {[e1*e2*e5 - e3*e4*e5], [e1*e3*e5], [e2*e3*e5], [e1*e4*e5], [e2*e4*e5]} over Rational Field

They look the same, but are different objects because they have been created with different generators.

Indeed:

sage: B.cohomology(3) is B.cohomology(3)
False
sage: B.cohomology(3) == B.cohomology(3)
False

comment:23 Changed 3 years ago by jhpalmieri

Can you add some of these as doctests, then?

comment:24 Changed 3 years ago by git

  • Commit changed from 88f8831fd8a04ce0e64b54e36e67a1f2ed1e6041 to 9da3f2ad9b6ef9e06f63a328fe6c015f4c4d63d9

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

9da3f2aAdd doctests for consistent cohomology groups

comment:25 Changed 3 years ago by mmarco

Doctests added.

comment:26 Changed 3 years ago by jhpalmieri

  • Branch changed from u/mmarco/check_for_formality_of_gcda_s to u/jhpalmieri/check_for_formality_of_gcda_s

comment:27 Changed 3 years ago by jhpalmieri

  • Commit changed from 9da3f2ad9b6ef9e06f63a328fe6c015f4c4d63d9 to e95c78ac7cbaffa8e17444b1dc309c2d1cf37a48
  • Reviewers changed from Travis Scrimshaw to Travis Scrimshaw, John Palmieri

Thank you. I've made a few small docstring fixes. If you're happy with my changes, please set to positive review on my behalf.


Last 10 new commits:

8a9a912Implemented numerical invariants and formality
90d6c50Auxiliar functions for cohomology class, and style fixes
5ec10caFix criterion for formality
b6d9f76Added some documentation and fixed style
2747034Style fixes
2a5d9b7Fixed pyflakes issues
3d8d8a4Make CohomologyClass CachedRepresentation
d953e3fFix order on for representation of cohomology classes
87b60c7Add doctests for consistent cohomology groups
e95c78atrac 28155: minor docstring fixes

comment:28 Changed 3 years ago by mmarco

  • Status changed from needs_review to positive_review

Thank you!

comment:29 Changed 3 years ago by vbraun

  • Branch changed from u/jhpalmieri/check_for_formality_of_gcda_s to e95c78ac7cbaffa8e17444b1dc309c2d1cf37a48
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:30 Changed 3 years ago by mmarco

  • Authors changed from Miguel Marco to Miguel Marco, Victor Manero
  • Commit e95c78ac7cbaffa8e17444b1dc309c2d1cf37a48 deleted
Note: See TracTickets for help on using tickets.