Opened 13 months ago

Closed 6 months ago

#27784 closed enhancement (fixed)

Characteristic Classes on Vector Bundles

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

Description (last modified by gh-DeRhamSource)

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 13 months ago by gh-DeRhamSource

  • Authors set to Michael Jung
  • Cc tscrim egourgoulhon added
  • Component changed from PLEASE CHANGE to geometry
  • Keywords characteristic classes manifolds added
  • Type changed from PLEASE CHANGE to enhancement

comment:2 Changed 13 months ago by gh-DeRhamSource

  • Description modified (diff)

comment:3 Changed 13 months ago by gh-DeRhamSource

  • Branch set to u/gh-DeRhamSource/characteristic_classes_on_manifolds

comment:4 Changed 12 months ago by git

  • Commit set to 716fb6975472331688cc108b524b80bebaef2360

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

comment:5 Changed 12 months ago by gh-DeRhamSource

  • Branch changed from u/gh-DeRhamSource/characteristic_classes_on_manifolds to u/gh-DeRhamSource/char_classes
  • Commit 716fb6975472331688cc108b524b80bebaef2360 deleted

comment:6 Changed 12 months ago by gh-DeRhamSource

  • Branch changed from u/gh-DeRhamSource/char_classes to u/gh-DeRhamSource/characteristic_classes_on_manifolds
  • Commit set to 716fb6975472331688cc108b524b80bebaef2360

comment:7 Changed 12 months ago by git

  • Commit changed from 716fb6975472331688cc108b524b80bebaef2360 to 963dede7873fc21626136606b1755767f7ceb241

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

963dedeis_field implemented

comment:8 Changed 12 months ago by embray

  • Milestone sage-8.8 deleted

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 11 months ago by gh-DeRhamSource

  • Milestone set to sage-8.9

comment:10 Changed 11 months ago by gh-DeRhamSource

  • Branch changed from u/gh-DeRhamSource/characteristic_classes_on_manifolds to u/gh-DeRhamSource/characteristic_classes
  • Commit changed from 963dede7873fc21626136606b1755767f7ceb241 to 2cabe93c84b8fe5aea82f46269f03e73b026cecd
  • Summary changed from Characteristic Classes on Manifolds to Characteristic Classes on Vector Bundles

New commits:

2cabe93General structure of vector bundles implemented

comment:11 Changed 8 months ago by gh-DeRhamSource

  • Dependencies set to #28159, #28189
  • Description modified (diff)
  • Milestone changed from sage-8.9 to sage-9.0

comment:12 Changed 8 months ago by gh-DeRhamSource

  • Description modified (diff)

comment:13 Changed 7 months ago by git

  • Commit changed from 2cabe93c84b8fe5aea82f46269f03e73b026cecd to b313f562826599d5361067df91606c32803a145a

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

b313f56Characteristic Classes

comment:14 Changed 7 months ago by gh-DeRhamSource

  • Dependencies changed from #28159, #28189 to #28159, #28189, #28564

comment:15 Changed 7 months ago by gh-DeRhamSource

  • Status changed from new to needs_review

comment:16 Changed 7 months ago by git

  • Commit changed from b313f562826599d5361067df91606c32803a145a to 0a2190fc1c0f7cf9e74a60f8ea3d590fdded30da

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

0a2190fTicket #27784: Wrong naming in Doctest

comment:17 Changed 7 months ago by git

  • Commit changed from 0a2190fc1c0f7cf9e74a60f8ea3d590fdded30da to 1bae0ab620ca652edcf75979583626c4bf82f438

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

1bae0abTrac #27784: Documentation grammar checkup

comment:18 Changed 7 months ago by git

  • Commit changed from 1bae0ab620ca652edcf75979583626c4bf82f438 to 32ecd26f6f0172c3a3ab0ec0f2928a37a9d72862

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 7 months ago by gh-DeRhamSource

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 7 months ago by gh-DeRhamSource (previous) (diff)

comment:20 Changed 7 months ago by git

  • Commit changed from 32ecd26f6f0172c3a3ab0ec0f2928a37a9d72862 to cacd4131916ff872077ef17f516a58a99f63474e

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

cacd413Trac #27784: Pyflake errors

comment:21 Changed 7 months ago by git

  • Commit changed from cacd4131916ff872077ef17f516a58a99f63474e to 2ec3a435fc3742a583f01a897ab393a4f514cf9c

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

2ec3a43Trac #27784: Whitspace removed

comment:22 Changed 7 months ago by git

  • Commit changed from 2ec3a435fc3742a583f01a897ab393a4f514cf9c to a78e1eca7caa78caf5d592ff9ed93843496294e1

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

a78e1ecTrac #27784: Documentation improved

comment:23 follow-up: Changed 7 months ago by tscrim

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 7 months ago by gh-DeRhamSource

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 follow-up: Changed 7 months ago by tscrim

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 7 months ago by git

  • Commit changed from a78e1eca7caa78caf5d592ff9ed93843496294e1 to 0857ec4dfd6027a8725712631d908a62ac49eef2

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 7 months ago by gh-DeRhamSource

Hopefully, I took everything into account you mentioned.

Thank you for the extensive feedback! :)

Last edited 7 months ago by gh-DeRhamSource (previous) (diff)

comment:28 Changed 7 months ago by git

  • Commit changed from 0857ec4dfd6027a8725712631d908a62ac49eef2 to 1db2c89cc0833f3889a97bcc73cad056872434d9

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

1db2c89Trac #27784: Line breaks

comment:29 follow-ups: Changed 7 months ago by 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)
    
  • 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 ; follow-up: Changed 7 months ago by gh-DeRhamSource

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 ; follow-up: Changed 7 months ago by gh-DeRhamSource

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

comment:32 in reply to: ↑ 30 Changed 7 months ago by egourgoulhon

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 7 months ago by egourgoulhon

Replying to gh-DeRhamSource:

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

Yes most probably

comment:34 Changed 7 months ago by git

  • Commit changed from 1db2c89cc0833f3889a97bcc73cad056872434d9 to ebbfb16bb285b1cb3a9430eae2a5165ba7e891e2

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 7 months ago by gh-DeRhamSource

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 7 months ago by gh-DeRhamSource (previous) (diff)

comment:36 Changed 7 months ago by egourgoulhon

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 7 months ago by git

  • Commit changed from ebbfb16bb285b1cb3a9430eae2a5165ba7e891e2 to 310cf9a302e28500c77f3cb0a2b23f440c43a155

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

310cf9aTrac #27784: set_restriction placed wrong

comment:38 Changed 7 months ago by gh-DeRhamSource

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 7 months ago by gh-DeRhamSource

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 ; follow-up: Changed 7 months ago by 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.

comment:41 follow-up: Changed 7 months ago by 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 7 months ago by gh-DeRhamSource

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 7 months ago by gh-DeRhamSource (previous) (diff)

comment:43 Changed 7 months ago by git

  • Commit changed from 310cf9a302e28500c77f3cb0a2b23f440c43a155 to 3fa8d85ed4e6a27388fcbe629e0dcea8017dbf47

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

3fa8d85Trac 27784: Tab character removed

comment:44 Changed 7 months ago by gh-DeRhamSource

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

comment:45 follow-up: Changed 7 months ago by tscrim

Patchbot has come back green.

comment:46 in reply to: ↑ 40 Changed 7 months ago by egourgoulhon

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 7 months ago by egourgoulhon

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 7 months ago by git

  • Commit changed from 3fa8d85ed4e6a27388fcbe629e0dcea8017dbf47 to e4bb884b04dd0ad2ebcbb2db4df72d3eb300ef50

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

e4bb884Trac #27784: File renamed + documentation

comment:49 follow-up: Changed 7 months ago by gh-DeRhamSource

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

comment:50 in reply to: ↑ 49 Changed 7 months ago by egourgoulhon

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 7 months ago by tscrim

  • Reviewers set to Travis Scrimshaw, Eric Gourgoulhon
  • Status changed from needs_review to positive_review

Yep, I am also good with it.

comment:52 Changed 7 months ago by git

  • Commit changed from e4bb884b04dd0ad2ebcbb2db4df72d3eb300ef50 to 3c93ab9fb44cf243f824373e6cc5c9707faae7e1
  • Status changed from positive_review to needs_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 7 months ago by gh-DeRhamSource

Please give it a positive review again...

comment:54 Changed 7 months ago by egourgoulhon

  • Status changed from needs_review to positive_review

The 9.0.beta6 patchbot is all green.

comment:55 Changed 7 months ago by git

  • Commit changed from 3c93ab9fb44cf243f824373e6cc5c9707faae7e1 to dba0660fb3319fadcc1482a1b8db637a47678c03
  • Status changed from positive_review to needs_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 follow-up: Changed 7 months ago by gh-DeRhamSource

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 7 months ago by egourgoulhon

  • Status changed from needs_review to positive_review

comment:58 in reply to: ↑ 56 Changed 7 months ago by egourgoulhon

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 6 months ago by git

  • Commit changed from dba0660fb3319fadcc1482a1b8db637a47678c03 to 6830d30b2c4aad7dd0c7b383eb8befa3d9c8bde8
  • Status changed from positive_review to needs_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 6 months ago by gh-DeRhamSource

  • Dependencies changed from #28159, #28189, #28564 to #28159, #28189, #28564, #28716

Please check the merge and doc change.

comment:61 Changed 6 months ago by egourgoulhon

  • Status changed from needs_review to positive_review

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

comment:62 Changed 6 months ago by vbraun

  • Branch changed from u/gh-DeRhamSource/characteristic_classes to 6830d30b2c4aad7dd0c7b383eb8befa3d9c8bde8
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.