sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.33k stars 453 forks source link

Characteristic Classes - MixedAlgebra #27584

Closed b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f closed 5 years ago

b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago

This project is my master thesis. I want to implement a computation of characteristic classes of vector bundles out of their curvature matrices. The algorithm is based on this short script.

In this first step, the graded algebra of "mixed" differential forms of a manifold will be introduced so one can use the matrix framework of sage in the following steps.

A demo notebook is provided at https://github.com/DeRhamSource/MixedFormNotebook

CC: @egourgoulhon @tscrim

Component: geometry

Keywords: characteristic classes

Author: Michael Jung

Branch/Commit: cdfaf6d

Reviewer: Eric Gourgoulhon, Travis Scrimshaw

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

b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago

Changed keywords from none to characteristic classes

b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago

Author: Michael Jung

b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago

Description changed:

--- 
+++ 
@@ -1 +1,3 @@
+This project is my master thesis. I want to implement a computation of characteristic classes of vector bundles out of their curvature matrices. The algorithm is based on [this](https://www.math.uni-potsdam.de/fileadmin/user_upload/Prof-Geometrie/Dokumente/Lehre/Lehrmaterialien/charakteristisch.pdf) short script.

+In a first step, the graded algebra of "mixed" differential forms of a manifold will be introduced so one can use the matrix framework of sage in the following steps.
b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,3 @@
 This project is my master thesis. I want to implement a computation of characteristic classes of vector bundles out of their curvature matrices. The algorithm is based on [this](https://www.math.uni-potsdam.de/fileadmin/user_upload/Prof-Geometrie/Dokumente/Lehre/Lehrmaterialien/charakteristisch.pdf) short script.

-In a first step, the graded algebra of "mixed" differential forms of a manifold will be introduced so one can use the matrix framework of sage in the following steps.
+In this first step, the graded algebra of "mixed" differential forms of a manifold will be introduced so one can use the matrix framework of sage in the following steps.
b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago

Branch: u/DeRhamSource/char_class_algebra

b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago

Changed branch from u/DeRhamSource/char_class_algebra to u/gh-DeRhamSource/char_class_algebra

b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago

Changed branch from u/gh-DeRhamSource/char_class_algebra to none

b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago

Branch: u/gh-DeRhamSource/characteristic_classes___mixedalgebra

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

49f21e5Initial version (without doctests)
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Commit: 49f21e5

egourgoulhon commented 5 years ago
comment:8

Here is the answer to the question raised in https://groups.google.com/d/msg/sage-devel/hwsQWuLN0nc/1_8LjLL4AgAJ, namely when a is a differential form (from previously existing DiffForm class) and A is a mixed form (as introduced in this ticket), why do we get

sage: a.__mul__(A)
Mixed differential form A/\a on the 2-dimensional differentiable manifold M

I think this results from a clash between (i) the operator __mul__ being used to denote the wedge product for mixed differential forms and (ii) the operator __mul__ being used to denote tensor product for differential forms. Indeed a.__mul__(A) calls the method __mul__ implemented in line 2169 of src/sage/manifolds/differentiable/tensorfield.py, which is devoted to the tensor product (the differential form a being considered as a tensor field of type (0,p), where p=degree(a)). As you can see in line 2269, if A does not belong to the class TensorField, one assumes that A is a scalar field (the only possible case until this ticket) and the code return A*a, i.e. A.wedge(a).

A possible solution could be to change the lines 2269-2271 of tensorfield.py to

if isinstance(other, MixedForm):
    return other.parent()(self)._mul_(other)
elif not isinstance(other, TensorField):
    # Multiplication by a scalar field or a number
    return other * self

A better way would be to redefine __mul__ in DiffForm, falling back to the tensor field version if other is not a mixed form.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 49f21e5 to 4ba165d

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

4ba165dSage Compatibility
egourgoulhon commented 5 years ago
comment:10

Your code looks nice, as well as the demo notebook!

Just a remark at this stage: you should not put any Jupyter notebook in the ticket's git branch. Indeed, when the ticket gets a positive review, the branch is merged into Sage's main branch, which must contain only source code (it is already very large just with sources). It is of course a good idea to share notebooks during the development process, but you should put them to a public repository (e.g. GitHub) and simply add a link to them in some comment on the ticket.

tscrim commented 5 years ago
comment:11

The other thing you could do is add the notebook file as an attachment (but note that you cannot delete it afterwards).

b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago
comment:12

Oh I'm sorry, I didn't realize that. How can I delete this file from the ticket?

In the next step, I will translate the notebook to english and add Doctests to the main files. Then, I guess, it's ready for review.

egourgoulhon commented 5 years ago
comment:13

Replying to @DeRhamSource:

Oh I'm sorry, I didn't realize that. How can I delete this file from the ticket?

Since the file is not very large, I guess it suffices to run

git rm scripts/MixedAlgebra.ipynb

before your next commit. Otherwise (i.e. for a large file), I would have recommended to delete the branch and to start a brand new one.

In the next step, I will translate the notebook to english and add Doctests to the main files. Then, I guess, it's ready for review.

OK very good!

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 4ba165d to 3ca14f2

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

3ca14f2Notebook removed + Minor Changes
b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago

Description changed:

--- 
+++ 
@@ -1,3 +1,5 @@
 This project is my master thesis. I want to implement a computation of characteristic classes of vector bundles out of their curvature matrices. The algorithm is based on [this](https://www.math.uni-potsdam.de/fileadmin/user_upload/Prof-Geometrie/Dokumente/Lehre/Lehrmaterialien/charakteristisch.pdf) short script.

 In this first step, the graded algebra of "mixed" differential forms of a manifold will be introduced so one can use the matrix framework of sage in the following steps.
+
+A demo notebook is provided at https://github.com/DeRhamSource/MixedFormNotebook
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

c13bd1dSome doctest examples added
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from 3ca14f2 to c13bd1d

b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago
comment:17

I added a link to my public github repo for the demo notebook in the description.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

eea8b18Added some doctests + minor changes
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from c13bd1d to eea8b18

b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago
comment:19

There is now a full doctest implemented in mixed_form_algebra.py. However, I've encountered another problem while coding the doctest of mixed_form.py. Namely an issue with coercions from the symbolic ring:

sage: M = Manifold(2, 'M')
sage: X.<x,y> = M.chart()
sage: A = M.mixed_form_algebra()
sage: A._element_constructor_(x).disp(X.frame())
[x] + [0] + [0]
sage: A(x).disp(X.frame())
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-84-7bd889258762> in <module>()
----> 1 A(x).disp(X.frame())

AttributeError: 'NoneType' object has no attribute 'disp'

How is that possible? Shouldn't A(.) invoke the method A._elementconstructor?

tscrim commented 5 years ago
comment:20

It does, but only after checking for a coercion. So my guess is something is going strange with the coercion. Posting the full traceback would be useful.

A few unrelated comments:

isinstance(comp, (int, Integer)) -> comp in ZZ because it also catches cases like 2/1 (which is in QQ).

A few of your example blocks should be TESTS:: and EXAMPLES:: when followed immediately by doctests, which should also be indentent 1 level.

b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago
comment:21

Thanks! :)

The snippet isinstance(comp, (int, Integer)) is adapted from diff_form_module.py. Should I apply your suggestion in that file as well for this branch, then?

More feedback and suggestions are very welcome! :)

Unfortunately, there is no error message or traceback:

sage: M = Manifold(2, 'M')
sage: X.<x,y> = M.chart()
sage: A = M.mixed_form_algebra()
sage: A(x)

The coercion is already known:

sage: M.mixed_form_algebra().has_coerce_map_from(SR)
True

So, I have really no idea why it won't work. But for scalar fields, it does:

sage: f = M.scalar_field_algebra()(x); f
Scalar field on the 2-dimensional differentiable manifold M
tscrim commented 5 years ago
comment:22

Hmm...so the output of A(x) really is None...that is very strange. What does coercion_model.explain(x, A) result in?

b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago
comment:23
sage: M = Manifold(2, 'M')
sage: X.<x,y> = M.chart()
sage: A = M.mixed_form_algebra()
sage: coercion_model.explain(x, A)
Coercion on left operand via
    Generic morphism:
      From: Symbolic Ring
      To:   Graded algebra Omega^*(M) of mixed differential forms on the 2-dimensional differentiable manifold M
Arithmetic performed after coercions.
Result lives in Graded algebra Omega^*(M) of mixed differential forms on the 2-dimensional differentiable manifold M
tscrim commented 5 years ago
comment:24

What is the result of

sage: phi = A.coerce_map_from_(SR)
sage: phi
sage: type(phi)
sage: phi(x)
b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago
comment:25
sage: M = Manifold(2, 'M')
sage: X.<x,y> = M.chart()
sage: A = M.mixed_form_algebra()
sage: phi = A.coerce_map_from(SR)
sage: phi
Generic morphism:
  From: Symbolic Ring
  To:   Graded algebra Omega^*(M) of mixed differential forms on the 2-dimensional differentiable manifold M
sage: type(phi)
<type 'sage.categories.morphism.SetMorphism'>
sage: phi(x)
sage: type(phi(x))
<type 'NoneType'>
tscrim commented 5 years ago
comment:26

That was what I was expecting. Basically what is happening is when it goes through the coercion framework, the result is None. So you need to look into that coercion morphism and what function it is calling and why that is not returning the proper object.

b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago
comment:27

I'm sorry: How do I manage that?

tscrim commented 5 years ago
comment:28

You will have to explore the properties of the morphism, such as phi._function, and maybe also add a print() statement or two for debugging in the _element_constructor_ to see what is happening.

egourgoulhon commented 5 years ago
comment:29

I think the issue arises because SR is the base ring of the algebra A:

sage: A.base_ring()
Symbolic Ring

If I am correct, in such a case, the coercion SR --> A is implemented via the algebra operation x * A.one(), not via _element_constructor_. In the present case, x * A.one() fails:

sage: x * A.one()
...
RuntimeError: BUG in map, returned None x <type 'sage.categories.morphism.SetMorphism'> Generic morphism:
  From: Symbolic Ring
  To:   Graded algebra Omega^*(M) of mixed differential forms on the 2-dimensional differentiable manifold M

To make it work, you should implement the method _lmul_ in class MixedForm and add the line

    if other is SR:
        return True

in MixedFormAlgebra._coerce_map_from_. At least, this is what is done for the algebras of scalar fields: see line 512 of src/sage/manifolds/scalarfield_algebra.py and the definition of _lmul_ in line 2449 of src/sage/manifolds/scalarfield.py.

b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago
comment:30

Yes, perfect. That solved the problem. Thanks! :)

tscrim commented 5 years ago
comment:31

Replying to @egourgoulhon:

To make it work, you should implement the method _lmul_ in class MixedForm and add the line

    if other is SR:
        return True

in MixedFormAlgebra._coerce_map_from_. At least, this is what is done for the algebras of scalar fields: see line 512 of src/sage/manifolds/scalarfield_algebra.py and the definition of _lmul_ in line 2449 of src/sage/manifolds/scalarfield.py.

I am a little worried about this. What about _rmul_? This also makes the dependence too strong on the base ring being SR. Can we use if other is self.base_ring() or something like that?

egourgoulhon commented 5 years ago
comment:32

Replying to @tscrim:

I am a little worried about this. What about _rmul_?

I am puzzled now about the scalar field algebra implementation. Why do we have _lmul_ and not _rmul_? Indeed, __rmul__ is the standard Python operator for reflected operands and __lmul__ does not exist in Python. What's exactly the role of _lmul_ in Sage? I guess it is there for noncommutative base rings but I could not find a good place in the documentation explaining this... In the present case, both for scalar field and mixed form algebras, we are dealing with a base ring that is commutative (since it is a field), so shouldn't we have only _rmul_?

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

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

0d772aeFull Doctest added
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 5 years ago

Changed commit from eea8b18 to 0d772ae

b220a61f-8dcb-4d62-9a53-d56a2b1bfc8f commented 5 years ago
comment:35

Replying to @egourgoulhon:

To make it work, you should implement the method _lmul_ in class MixedForm and add the line

    if other is SR:
        return True

in MixedFormAlgebra._coerce_map_from_. At least, this is what is done for the algebras of scalar fields: see line 512 of src/sage/manifolds/scalarfield_algebra.py and the definition of _lmul_ in line 2449 of src/sage/manifolds/scalarfield.py.

I managed the coercion detection via

        if self._domain.scalar_field_algebra().has_coerce_map_from(S):
            return True

Does that also eliminate your doubts concerning the strong dependencies on SR, Travis?

egourgoulhon commented 5 years ago
comment:36

Replying to @egourgoulhon:

Replying to @tscrim:

I am a little worried about this. What about _rmul_?

I am puzzled now about the scalar field algebra implementation. Why do we have _lmul_ and not _rmul_? Indeed, __rmul__ is the standard Python operator for reflected operands and __lmul__ does not exist in Python. What's exactly the role of _lmul_ in Sage? I guess it is there for noncommutative base rings but I could not find a good place in the documentation explaining this... In the present case, both for scalar field and mixed form algebras, we are dealing with a base ring that is commutative (since it is a field), so shouldn't we have only _rmul_?

I see in line 2394 of src/sage/structure/element.pyx that _rmul_ falls back to _lmul_, which is not implemented at the ModuleElement level (actually returns None). This leaves the impression that when implementing a new module element class over a commutative ring, one shall actually implement _lmul_ and not _rmul_... Accordingly, my understanding at the moment is that in Sage, we have

Travis, do you confirm?

tscrim commented 5 years ago
comment:37

Replying to @egourgoulhon:

Replying to @egourgoulhon:

Replying to @tscrim:

I am a little worried about this. What about _rmul_?

I am puzzled now about the scalar field algebra implementation. Why do we have _lmul_ and not _rmul_? Indeed, __rmul__ is the standard Python operator for reflected operands and __lmul__ does not exist in Python. What's exactly the role of _lmul_ in Sage? I guess it is there for noncommutative base rings but I could not find a good place in the documentation explaining this... In the present case, both for scalar field and mixed form algebras, we are dealing with a base ring that is commutative (since it is a field), so shouldn't we have only _rmul_?

I see in line 2394 of src/sage/structure/element.pyx that _rmul_ falls back to _lmul_, which is not implemented at the ModuleElement level (actually returns None). This leaves the impression that when implementing a new module element class over a commutative ring, one shall actually implement _lmul_ and not _rmul_... Accordingly, my understanding at the moment is that in Sage, we have

  • _mul_(self, other) when self and other have the same parent
  • _lmul_(self, other) when self and other have distinct parents
  • _rmul_(self, other) is inherited from ModuleElement (if not redefined) and falls back to _lmul_

Travis, do you confirm?

Ah, right, I forgot that generic _rmul_ was implemented in ModuleElement.

So Sage's _rmul_ should not be confused with Python's __rmul__.

Python's __rmul__ is for when self knows how to multiply with other when self is on the right, but other does not. For instance, it handles 5 * self because we cannot override the int type (which means the "standard int.__mul__" fails), but self knows what to do.

Sage's _rmul_ and _lmul_ play the same role, just on different sides. They are there to implement actions of other on self on the right or left, and thus are used by the coercion framework (in particular, they check if coercions can be applied).

tscrim commented 5 years ago
comment:38

Replying to @DeRhamSource:

Replying to @egourgoulhon:

To make it work, you should implement the method _lmul_ in class MixedForm and add the line

    if other is SR:
        return True

in MixedFormAlgebra._coerce_map_from_. At least, this is what is done for the algebras of scalar fields: see line 512 of src/sage/manifolds/scalarfield_algebra.py and the definition of _lmul_ in line 2449 of src/sage/manifolds/scalarfield.py.

I managed the coercion detection via

        if self._domain.scalar_field_algebra().has_coerce_map_from(S):
            return True

Does that also eliminate your doubts concerning the strong dependencies on SR, Travis?

Yes, I think that is better. Generally, I do not think it is a good idea to hardcode a ring unless you really only want to work with that ring (and my understanding is that this could be made more flexible in the future).

egourgoulhon commented 5 years ago

Changed branch from u/gh-DeRhamSource/characteristic_classes___mixedalgebra to public/manifolds/mixed_differential_forms

egourgoulhon commented 5 years ago

Changed commit from 0d772ae to 506ef37

egourgoulhon commented 5 years ago
comment:39

There was a merge conflict with Sage 8.8.beta1 in the file src/sage/manifolds/differentiable/manifold.py. This was due to #27581 (initialization of the components of a tensor field while declaring it), which changed that file. The conflict is solved in the attached branch (since I am the author of #27581, it was probably easier that I solved this conflict, hence the new branch).


New commits:

506ef37Merge branch 'u/gh-DeRhamSource/characteristic_classes___mixedalgebra' of git://trac.sagemath.org/sage into Sage 8.8.beta1
egourgoulhon commented 5 years ago

Reviewer: Eric Gourgoulhon, Travis Scrimshaw

egourgoulhon commented 5 years ago
comment:40

If you click on the patchbot button (marked "8.8.beta1") located at the right of the ticket title, you will access to the patchbot reports. One of them is useless, due to some patchbot issue (the one marked "BuildFailed"). The most relevant one is from the patchbot "zancara". It indicates "TestFailed", but if you click either on "log" or "shortlog", you'll see that the failure occurs in a source file that has not been touched by this ticket: src/sage/tests/books/computational-mathematics-with-sagemath/graphique_doctest.py. So this is more an issue on the patchbot size. The only relevant issue is the pyflakes error marked by a red cross in the plugin list at the right of the summary of zancara's report: if you click on "diff", you get

src/sage/manifolds/differentiable/mixed_form_algebra.py:31: 
'sage.rings.integer.Integer' imported but unused

Can you please correct this? Note that you shall update your Sage version to 8.8.beta1. If this is not done already, it suffices to pull the new ticket branch and run MAKE="make -j8" make.