Opened 4 years ago

Closed 3 years ago

#27784 closed enhancement (fixed)

Characteristic Classes on Vector Bundles

Reported by: Michael Jung Owned by:
Priority: major Milestone: sage-9.0
Component: geometry Keywords: characteristic classes, manifolds
Cc: Travis Scrimshaw, Eric Gourgoulhon Merged in:
Authors: Michael Jung Reviewers: Travis Scrimshaw, Eric Gourgoulhon
Report Upstream: N/A Work issues:
Branch: 6830d30 (Commits, GitHub, GitLab) Commit: 6830d30b2c4aad7dd0c7b383eb8befa3d9c8bde8
Dependencies: #28159, #28189, #28564, #28716 Stopgaps:

Status badges

Description (last modified by Michael Jung)

Characteristic classes on differentiable vector bundles.

Features

  • characteristic classes on differentiable manifolds using the Chern-Weil homomorphism
  • class BundleConnection introduced with rudimentary methods
  • each characteristic class carries a dictionary with bundle connection as keys and mixed forms as values in order to simulate the cohomology equivalence

Planned sometime

  • transgression forms

Change History (62)

comment:1 Changed 4 years ago by Michael Jung

Authors: Michael Jung
Cc: Travis Scrimshaw Eric Gourgoulhon added
Component: PLEASE CHANGEgeometry
Keywords: characteristic classes manifolds added
Type: PLEASE CHANGEenhancement

comment:2 Changed 4 years ago by Michael Jung

Description: modified (diff)

comment:3 Changed 4 years ago by Michael Jung

Branch: u/gh-DeRhamSource/characteristic_classes_on_manifolds

comment:4 Changed 3 years ago by git

Commit: 716fb6975472331688cc108b524b80bebaef2360

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

comment:5 Changed 3 years ago by Michael Jung

Branch: u/gh-DeRhamSource/characteristic_classes_on_manifoldsu/gh-DeRhamSource/char_classes
Commit: 716fb6975472331688cc108b524b80bebaef2360

comment:6 Changed 3 years ago by Michael Jung

Branch: u/gh-DeRhamSource/char_classesu/gh-DeRhamSource/characteristic_classes_on_manifolds
Commit: 716fb6975472331688cc108b524b80bebaef2360

comment:7 Changed 3 years ago by git

Commit: 716fb6975472331688cc108b524b80bebaef2360963dede7873fc21626136606b1755767f7ceb241

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

963dedeis_field implemented

comment:8 Changed 3 years ago by Erik Bray

Milestone: sage-8.8

As the Sage-8.8 release milestone is pending, we should delete the sage-8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage-8.9).

comment:9 Changed 3 years ago by Michael Jung

Milestone: sage-8.9

comment:10 Changed 3 years ago by Michael Jung

Branch: u/gh-DeRhamSource/characteristic_classes_on_manifoldsu/gh-DeRhamSource/characteristic_classes
Commit: 963dede7873fc21626136606b1755767f7ceb2412cabe93c84b8fe5aea82f46269f03e73b026cecd
Summary: Characteristic Classes on ManifoldsCharacteristic Classes on Vector Bundles

New commits:

2cabe93General structure of vector bundles implemented

comment:11 Changed 3 years ago by Michael Jung

Dependencies: #28159, #28189
Description: modified (diff)
Milestone: sage-8.9sage-9.0

comment:12 Changed 3 years ago by Michael Jung

Description: modified (diff)

comment:13 Changed 3 years ago by git

Commit: 2cabe93c84b8fe5aea82f46269f03e73b026cecdb313f562826599d5361067df91606c32803a145a

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

b313f56Characteristic Classes

comment:14 Changed 3 years ago by Michael Jung

Dependencies: #28159, #28189#28159, #28189, #28564

comment:15 Changed 3 years ago by Michael Jung

Status: newneeds_review

comment:16 Changed 3 years ago by git

Commit: b313f562826599d5361067df91606c32803a145a0a2190fc1c0f7cf9e74a60f8ea3d590fdded30da

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

0a2190fTicket #27784: Wrong naming in Doctest

comment:17 Changed 3 years ago by git

Commit: 0a2190fc1c0f7cf9e74a60f8ea3d590fdded30da1bae0ab620ca652edcf75979583626c4bf82f438

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

1bae0abTrac #27784: Documentation grammar checkup

comment:18 Changed 3 years ago by git

Commit: 1bae0ab620ca652edcf75979583626c4bf82f43832ecd26f6f0172c3a3ab0ec0f2928a37a9d72862

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

32ecd26Trac #27784: 0-th element of Pfaffian class is always zero

comment:19 Changed 3 years ago by Michael Jung

I know, you are really busy right now, Eric. However, this ticket is the essence and foundation of my master thesis and I'd highly appreciate at least a short look whether the structure is okay.

Travis, could you take a short look, too?

Details can be changed later, but the rough structure should be fine before I start writing my thesis.

Thank you both! I really appreciate your effort so far. :)

Last edited 3 years ago by Michael Jung (previous) (diff)

comment:20 Changed 3 years ago by git

Commit: 32ecd26f6f0172c3a3ab0ec0f2928a37a9d72862cacd4131916ff872077ef17f516a58a99f63474e

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

cacd413Trac #27784: Pyflake errors

comment:21 Changed 3 years ago by git

Commit: cacd4131916ff872077ef17f516a58a99f63474e2ec3a435fc3742a583f01a897ab393a4f514cf9c

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

2ec3a43Trac #27784: Whitspace removed

comment:22 Changed 3 years ago by git

Commit: 2ec3a435fc3742a583f01a897ab393a4f514cf9ca78e1eca7caa78caf5d592ff9ed93843496294e1

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

a78e1ecTrac #27784: Documentation improved

comment:23 Changed 3 years ago by Travis Scrimshaw

I won't be able to look at it for at least a few days. However, I will make it a high priority.

comment:24 in reply to:  23 Changed 3 years ago by Michael Jung

Replying to tscrim:

I won't be able to look at it for at least a few days. However, I will make it a high priority.

Thank you so much! :)

comment:25 Changed 3 years ago by Travis Scrimshaw

Here are my comments (mostly on the technical details):

Don't feel the strict need to adhere to PEP8 when it degrades readability. In particular, don't have a line break here:

-from sage.manifolds.differentiable.vector_bundle\
-                                               import DifferentiableVectorBundle
+from sage.manifolds.differentiable.vector_bundle import DifferentiableVectorBundle

and similarly here

            sage: from sage.manifolds.differentiable.bundle_connection import \
                                                                BundleConnection

In contrast, you can break .. MATH:: lines.

Remove the period/full-stop at the end here:

    - ``latex_name`` -- (default: ``None``) LaTeX symbol to denote the bundle
      connection; if ``None``, it is set to ``name``.

I don't see the point of this note:

    .. NOTE::

        Notice that this is a *very* rudimentary form of bundle connections.
        A more detailed implementation is devoted to :trac:`28640`.

The only people who will care are developers, it is irrelevant to the use of this class, and it makes it come across as less polished.

The block after They certainly obey the structure equation:: is over-indented.

A general comment (not something you need to address here, but I noticed it now): we should have a little mix-in class for _latex_name objects (or perhaps more generally NamedObject) so we don't have so much repeated boilerplate code.

Instead of _hash_value, the more standard naming is _hash.

Error messages should follow the general Python convention:

-raise ValueError("A frame must be provided!")
+raise ValueError("a frame must be provided")

Is this relevant? # TODO: Compute transformations Should it instead raise a NotImplementedError?

Is there some way we can automate this?

        .. WARNING::

            If the connection has already forms in other frames, it is the
            user's responsibility to make sure that the 1-forms to be added
            are consistent with them.
-computation in SAGE.
+computation in Sage.

Although really I would suggest just deleting that sentence (you already are using Sage and such an internal working, if you insist on including it, should be in an ALGORITHM: section).

I generally dislike these things:

Class Documentation
-------------------

They don't convey any information and just are a bit of a distraction IMO.

_get_predefined_class is not a static method (it is just a separate function). Also `arg` -> ``arg``

This should just be an if-else statement:

    c_dict = {}
    c_dict['ChernChar'] = ('complex', 'additive', 'ch', r'\mathrm{ch}', x.exp())
    c_dict['Todd'] = ('complex', 'additive', 'Td', r'\mathrm{Td}',
                      x / (1 - (-x).exp()))
    c_dict['Chern'] = ('complex', 'multiplicative', 'c', 'c', 1 + x)
    c_dict['Pontryagin'] = ('real', 'multiplicative', 'p', 'p', 1 + x)
    c_dict['AHat'] = ('real', 'multiplicative', 'A^', r'\hat{A}',
                      x.sqrt() / (2 * (x.sqrt() / 2).sinh()))
    c_dict['Hirzebruch'] = ('real', 'multiplicative', 'L', 'L',
                            (x.sqrt() / x.sqrt().tanh()))
    c_dict['Euler'] = ('real', 'Pfaffian', 'e', 'e', x)
    # Get class from arg
    try:
        res = c_dict[arg]
    except KeyError:
        raise ValueError("the characteristic class '{}' is ".format(arg) +
                         "not predefined yet.")

unless you were hoping to cache c_dict, but I don't see the utility in that as this should not be a bottleneck.

Do not abbreviate class names and UniqueRepresentation is better being first in the MRO:

-class CharClass(SageObject, UniqueRepresentation):
+class CharacteristicClass(UniqueRepresentation, SageObject):

IMO, generally you should avoid abbreviated names for inputs/function names/etc. for the main implementation (although it is okay to have an abbreviated alias). (For another example, get_form(self, con, cmatrices=None): and use connection instead of con and def char_class to def characteristic_class) This better self-documents the code and can make things more discoverable with tab completion. For example, I might type .charact<tab> and then be surprised that nothing comes up.

In _get_coeff_list, there is no need for the coef variable (just return the result). Also

-(self._base_space._dim / 2).floor()
+self._base_space._dim // 2

and make from sage.symbolic.ring import SR at the top-level.

-- ``cmatrix`` curvature matrix
+- ``cmatrix`` -- curvature matrix

comment:26 Changed 3 years ago by git

Commit: a78e1eca7caa78caf5d592ff9ed93843496294e10857ec4dfd6027a8725712631d908a62ac49eef2

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

47e2f51copy method with naming
f55b881Doctest adapted
f85de87Merge branch 'develop' into t/28564/tensor_fields__consistent_naming
eb92e6aDoctest fixed
5b2eee2'copy' with individual naming
38aa7f5Merge branch 'develop' into t/28564/tensor_fields__consistent_naming
9d39cb5Merge branch 'develop' into t/28564/tensor_fields__consistent_naming
67e5cdcTrac #28564: One line doctest error fixed
38eeb4aMerge branch 't/28564/tensor_fields__consistent_naming' into char_classes-27784
0857ec4Trac #27784: Typos and minor code improvements

comment:27 Changed 3 years ago by Michael Jung

Hopefully, I took everything into account you mentioned.

Version 0, edited 3 years ago by Michael Jung (next)

comment:28 Changed 3 years ago by git

Commit: 0857ec4dfd6027a8725712631d908a62ac49eef21db2c89cc0833f3889a97bcc73cad056872434d9

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

1db2c89Trac #27784: Line breaks

comment:29 Changed 3 years ago by Eric Gourgoulhon

I gave a quik look. It looks nice! Thanks for having taken into account Travis' remarks. I fully agree with them. Here a few remarks from my side:

  • There is a failed doctest:
    File "src/sage/manifolds/differentiable/char_class.py", line 685, in sage.manifolds.differentiable.char_class.CharacteristicClass.get_form
    Failed example:
        ch_form.display()
    Expected:
        ch(E, nabla^E) = ch_0(E, nabla^E) + zero + ch_1(E, nabla^E)
    Got:
        ch(E, nabla^E) = ch_0.0(E, nabla^E) + zero + ch_1.0(E, nabla^E)
    
  • There are two warnings when generating the documentation:
    [manifolds] <unknown>:2566: DeprecationWarning: invalid escape sequence \c
    [manifolds] <unknown>:2565: DeprecationWarning: invalid escape sequence \c
    
  • in the OUTPUT section of BundleConnection.add_connection_form(), a back quote is missing:
    - connection 1-form `\omega^i_j in the given frame, as an instance of
    + connection 1-form `\omega^i_j` in the given frame, as an instance of
    
  • in the introduction to characteristic classes, N is not defined in the sentence
    for any continuous map `f: M \to N`
    
  • in the doctest example of in DifferentiableVectorBundle.characteristic_class():
    - sage: g = M.metric('g')
    + sage: g = M.metric()
    

(since M has been defined a Lorentzian manifold, g has already been initialized, with the name 'g')

comment:30 in reply to:  29 ; Changed 3 years ago by Michael Jung

Replying to egourgoulhon:

I gave a quik look. It looks nice! Thanks for having taken into account Travis' remarks. I fully agree with them. Here a few remarks from my side:

  • There is a failed doctest:
    File "src/sage/manifolds/differentiable/char_class.py", line 685, in sage.manifolds.differentiable.char_class.CharacteristicClass.get_form
    Failed example:
        ch_form.display()
    Expected:
        ch(E, nabla^E) = ch_0(E, nabla^E) + zero + ch_1(E, nabla^E)
    Got:
        ch(E, nabla^E) = ch_0.0(E, nabla^E) + zero + ch_1.0(E, nabla^E)
    

That's strange...I don't get this error. Travis?

The rest is in progress...

comment:31 in reply to:  29 ; Changed 3 years ago by Michael Jung

Ah, that might come from using Python 2.7. Hang on there.

comment:32 in reply to:  30 Changed 3 years ago by Eric Gourgoulhon

Replying to gh-DeRhamSource:

That's strange...I don't get this error. Travis?

Are you using Python 3 Sage ?

comment:33 in reply to:  31 Changed 3 years ago by Eric Gourgoulhon

Replying to gh-DeRhamSource:

Ah, that might come from using Python 2.7. Hang on there.

Yes most probably

comment:34 Changed 3 years ago by git

Commit: 1db2c89cc0833f3889a97bcc73cad056872434d9ebbfb16bb285b1cb3a9430eae2a5165ba7e891e2

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

5dd53b0Trac #28564: raw strings for LaTeX code
2e15f7fMerge branch 't/28564/tensor_fields__consistent_naming' into char_classes-27784
ebbfb16Trac #27784: manif zero forms set to unique zero + int division + documentation

comment:35 Changed 3 years ago by Michael Jung

The issue about the DeprecationWarning referred to #28564. That has been fixed.

The other issues has been solved, too.

However, I'll build Sage from scratch, now.

Last edited 3 years ago by Michael Jung (previous) (diff)

comment:36 Changed 3 years ago by Eric Gourgoulhon

With the latest version, there are two failed doctests in char_class.py. I got them on my machine and they are reported by the patchbot as well.

comment:37 Changed 3 years ago by git

Commit: ebbfb16bb285b1cb3a9430eae2a5165ba7e891e2310cf9a302e28500c77f3cb0a2b23f440c43a155

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

310cf9aTrac #27784: set_restriction placed wrong

comment:38 Changed 3 years ago by Michael Jung

I guess that should be it. However, my build is not finished yet. I will start an extensive test as soon as possible.

Thanks so far! :)

comment:39 Changed 3 years ago by Michael Jung

My tests did all pass. Patchbot says so, too.

By the way: Why is the patchbot yellow and not green though all tests passed?

comment:40 in reply to:  25 ; Changed 3 years ago by Michael Jung

Replying to tscrim:

Is there some way we can automate this?

        .. WARNING::

            If the connection has already forms in other frames, it is the
            user's responsibility to make sure that the 1-forms to be added
            are consistent with them.

The concept and warning is just copied from affine_connection.py. At least it should be possible to return an error message when some incompatibilities occur on the intersection. But that's a topic for another ticket, I'd say.

comment:41 Changed 3 years ago by Frédéric Chapoton

Patchbot says:

sage -t --long src/sage/parallel/map_reduce.py  # Timed out
sage -t --long src/doc/en/reference/references/index.rst  # Tab character found

comment:42 in reply to:  41 Changed 3 years ago by Michael Jung

Replying to chapoton:

Patchbot says:

sage -t --long src/sage/parallel/map_reduce.py  # Timed out
sage -t --long src/doc/en/reference/references/index.rst  # Tab character found

The first one has nothing to do with the changes in this ticket.

However, the second has been fixed now.

Thanks! :)

Last edited 3 years ago by Michael Jung (previous) (diff)

comment:43 Changed 3 years ago by git

Commit: 310cf9a302e28500c77f3cb0a2b23f440c43a1553fa8d85ed4e6a27388fcbe629e0dcea8017dbf47

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

3fa8d85Trac 27784: Tab character removed

comment:44 Changed 3 years ago by Michael Jung

Also, it seems there is a doctest missing in map_reduce.py. What's going on there? :-/

comment:45 Changed 3 years ago by Travis Scrimshaw

Patchbot has come back green.

comment:46 in reply to:  40 Changed 3 years ago by Eric Gourgoulhon

Replying to gh-DeRhamSource:

Replying to tscrim:

Is there some way we can automate this?

        .. WARNING::

            If the connection has already forms in other frames, it is the
            user's responsibility to make sure that the 1-forms to be added
            are consistent with them.

The concept and warning is just copied from affine_connection.py. At least it should be possible to return an error message when some incompatibilities occur on the intersection. But that's a topic for another ticket, I'd say.

I agree: this could be postponed to another ticket; in particular, checking the compatibility can be computationaly demanding and one shall discuss the appropriate way to do this, with possibly an option check=True or check=False.

comment:47 in reply to:  45 Changed 3 years ago by Eric Gourgoulhon

Replying to tscrim:

Patchbot has come back green.

From my side, I think we are almost done here. Two last remarks:

  • in the introduction to characteristic classes, \mathfrak{g} is not defined
  • could the file char_class.py be renamed characteristic_class.py? (this would improve the readability of the directory src/sage/manifolds/differentiable and would match with the name of the class CharacteristicClass)

comment:48 Changed 3 years ago by git

Commit: 3fa8d85ed4e6a27388fcbe629e0dcea8017dbf47e4bb884b04dd0ad2ebcbb2db4df72d3eb300ef50

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

e4bb884Trac #27784: File renamed + documentation

comment:49 Changed 3 years ago by Michael Jung

I hope, I took the renaming everywhere into account...

comment:50 in reply to:  49 Changed 3 years ago by Eric Gourgoulhon

Replying to gh-DeRhamSource:

I hope, I took the renaming everywhere into account...

Thanks for the changes. I am OK for a positive review. Travis?

comment:51 Changed 3 years ago by Travis Scrimshaw

Reviewers: Travis Scrimshaw, Eric Gourgoulhon
Status: needs_reviewpositive_review

Yep, I am also good with it.

comment:52 Changed 3 years ago by git

Commit: e4bb884b04dd0ad2ebcbb2db4df72d3eb300ef503c93ab9fb44cf243f824373e6cc5c9707faae7e1
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

3c93ab9Trac 27784: Merge branch 'develop' into char_classes-27784

comment:53 Changed 3 years ago by Michael Jung

Please give it a positive review again...

comment:54 Changed 3 years ago by Eric Gourgoulhon

Status: needs_reviewpositive_review

The 9.0.beta6 patchbot is all green.

comment:55 Changed 3 years ago by git

Commit: 3c93ab9fb44cf243f824373e6cc5c9707faae7e1dba0660fb3319fadcc1482a1b8db637a47678c03
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

dba0660Trac #27784: Rank defined in documentation

comment:56 Changed 3 years ago by Michael Jung

That was just a very very minor thing I noticed during my thesis preparation. Sorry.

By the way: Am I allowed to switch the review status back to "positive" by myself in such cases?

comment:57 Changed 3 years ago by Eric Gourgoulhon

Status: needs_reviewpositive_review

comment:58 in reply to:  56 Changed 3 years ago by Eric Gourgoulhon

Replying to gh-DeRhamSource:

That was just a very very minor thing I noticed during my thesis preparation. Sorry.

No problem. It's always good to improve the documentation!

By the way: Am I allowed to switch the review status back to "positive" by myself in such cases?

I don't think we have very precise rules about this. IMHO, it seems better to have an external eye, even for seemingly minor changes.

comment:59 Changed 3 years ago by git

Commit: dba0660fb3319fadcc1482a1b8db637a47678c036830d30b2c4aad7dd0c7b383eb8befa3d9c8bde8
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

a331374Trac #27784: `g(0)=0` condition for real additive classes added in doc
013fb8bAdd construction of a vector frame from a family of vector fields
71b5f0fReplace keyword argument 'from_family' by positional argument in vector_frame()
d0ef4d7Add construction of a local frame from a set of vector fields in TensorBundle.local_frame()
6635fadConstruction of a local frame on a vector bundle from a family of sections
5c9c516Merge branch 'public/manifolds/vector_frame_from_family-28716' of git://trac.sagemath.org/sage into Sage 9.0.beta6
81e2f60Minor fix in the documentation of TopologicalVectorBundle.local_frame()
6830d30Trac #27784: Merge trac #28716 into trac #27784

comment:60 Changed 3 years ago by Michael Jung

Dependencies: #28159, #28189, #28564#28159, #28189, #28564, #28716

Please check the merge and doc change.

comment:61 Changed 3 years ago by Eric Gourgoulhon

Status: needs_reviewpositive_review

Just checked that everything is in order with the new depedency #28716 and that all doctests are passed.

comment:62 Changed 3 years ago by Volker Braun

Branch: u/gh-DeRhamSource/characteristic_classes6830d30b2c4aad7dd0c7b383eb8befa3d9c8bde8
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.