Opened 3 years ago

Closed 3 years ago

# Characteristic Classes - MixedAlgebra

Reported by: Owned by: gh-DeRhamSource major sage-8.8 geometry characteristic classes, manifolds egourgoulhon, tscrim Michael Jung Eric Gourgoulhon, Travis Scrimshaw N/A cdfaf6d cdfaf6d16322ef4c7d014680ebf025f272f62eb6

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

### comment:1 Changed 3 years ago by gh-DeRhamSource

• Authors set to Michael Jung
• Component changed from PLEASE CHANGE to geometry
• Description modified (diff)
• Keywords characteristic classes manifolds added
• Type changed from PLEASE CHANGE to enhancement

### comment:2 Changed 3 years ago by gh-DeRhamSource

• Description modified (diff)
• Summary changed from Characteristic Classes to Characteristic Classes - MixedAlgebra

### comment:3 Changed 3 years ago by gh-DeRhamSource

• Branch set to u/DeRhamSource/char_class_algebra

### comment:4 Changed 3 years ago by gh-DeRhamSource

• Branch changed from u/DeRhamSource/char_class_algebra to u/gh-DeRhamSource/char_class_algebra

### comment:5 Changed 3 years ago by gh-DeRhamSource

• Branch u/gh-DeRhamSource/char_class_algebra deleted

### comment:6 Changed 3 years ago by gh-DeRhamSource

• Branch set to u/gh-DeRhamSource/characteristic_classes___mixedalgebra

### comment:7 Changed 3 years ago by git

• Commit set to 49f21e50d792f4000c0b0b6cf64c93fd5eaf2da2

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

 ​49f21e5 Initial version (without doctests)

### comment:8 Changed 3 years ago by egourgoulhon

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.

### comment:9 Changed 3 years ago by git

• Commit changed from 49f21e50d792f4000c0b0b6cf64c93fd5eaf2da2 to 4ba165d85dff93b5cbad4fb7af3b403e2e968077

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

 ​4ba165d Sage Compatibility

### comment:10 Changed 3 years ago by egourgoulhon

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.

### comment:11 Changed 3 years ago by tscrim

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

### comment:12 follow-up: ↓ 13 Changed 3 years ago by gh-DeRhamSource

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.

Last edited 3 years ago by gh-DeRhamSource (previous) (diff)

### comment:13 in reply to: ↑ 12 Changed 3 years ago by egourgoulhon

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!

### comment:14 Changed 3 years ago by git

• Commit changed from 4ba165d85dff93b5cbad4fb7af3b403e2e968077 to 3ca14f2de40c60ec79036c03931dc5b30729cee6

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

 ​3ca14f2 Notebook removed + Minor Changes

### comment:15 Changed 3 years ago by gh-DeRhamSource

• Description modified (diff)

### comment:16 Changed 3 years ago by git

• Commit changed from 3ca14f2de40c60ec79036c03931dc5b30729cee6 to c13bd1dde3a4c5c881c73d734a2b46285ed42074

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

 ​c13bd1d Some doctest examples added

### comment:17 Changed 3 years ago by gh-DeRhamSource

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

### comment:18 Changed 3 years ago by git

• Commit changed from c13bd1dde3a4c5c881c73d734a2b46285ed42074 to eea8b18ed4e00f01a1f67bd2694200c9a2919006

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

 ​eea8b18 Added some doctests + minor changes

### comment:19 Changed 3 years ago by gh-DeRhamSource

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._element_constructor_?

### comment:20 Changed 3 years ago by tscrim

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.

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.

### comment:21 Changed 3 years ago by gh-DeRhamSource

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)


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

Last edited 3 years ago by gh-DeRhamSource (previous) (diff)

### comment:22 Changed 3 years ago by tscrim

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

### comment:23 Changed 3 years ago by gh-DeRhamSource

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


### comment:24 Changed 3 years ago by tscrim

What is the result of

sage: phi = A.coerce_map_from_(SR)
sage: phi
sage: type(phi)
sage: phi(x)


### comment:25 Changed 3 years ago by gh-DeRhamSource

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'>


### comment:26 Changed 3 years ago by tscrim

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.

### comment:27 Changed 3 years ago by gh-DeRhamSource

I'm sorry: How do I manage that?

### comment:28 Changed 3 years ago by tscrim

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.

### comment:29 follow-ups: ↓ 31 ↓ 35 Changed 3 years ago by egourgoulhon

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.

### comment:30 Changed 3 years ago by gh-DeRhamSource

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

### comment:31 in reply to: ↑ 29 ; follow-up: ↓ 32 Changed 3 years ago by tscrim

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?

### comment:32 in reply to: ↑ 31 ; follow-up: ↓ 36 Changed 3 years ago by egourgoulhon

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_?

### comment:33 Changed 3 years ago by git

• Commit changed from eea8b18ed4e00f01a1f67bd2694200c9a2919006 to 0d772ae29e7971ab8f5549eb5e647dde66b2ba9d

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

 ​0d772ae Full Doctest added

### comment:34 Changed 3 years ago by gh-DeRhamSource

• Status changed from new to needs_review

### comment:35 in reply to: ↑ 29 ; follow-up: ↓ 38 Changed 3 years ago by gh-DeRhamSource

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?

### comment:36 in reply to: ↑ 32 ; follow-up: ↓ 37 Changed 3 years ago by egourgoulhon

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?

Last edited 3 years ago by egourgoulhon (previous) (diff)

### comment:37 in reply to: ↑ 36 Changed 3 years ago by 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).

### comment:38 in reply to: ↑ 35 Changed 3 years ago by tscrim

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).

### comment:39 Changed 3 years ago by egourgoulhon

• Branch changed from u/gh-DeRhamSource/characteristic_classes___mixedalgebra to public/manifolds/mixed_differential_forms
• Commit changed from 0d772ae29e7971ab8f5549eb5e647dde66b2ba9d to 506ef37791cdd89b1b2fa9b8009827197205c17e

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:

 ​506ef37 Merge branch 'u/gh-DeRhamSource/characteristic_classes___mixedalgebra' of git://trac.sagemath.org/sage into Sage 8.8.beta1
Last edited 3 years ago by egourgoulhon (previous) (diff)

### comment:40 Changed 3 years ago by egourgoulhon

• Reviewers set to Eric Gourgoulhon, Travis Scrimshaw

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.

### comment:41 follow-up: ↓ 43 Changed 3 years ago by egourgoulhon

Regarding the documentation: you have added doctests in the Python sources but the documentation is not generated since there is no entry to it in the sources of the reference manual. You shall modify/add files in src/doc/en/reference/manifolds, for instance in a way similar to what is done for differential forms, i.e. create a file mixed_diff_form.rst and add a pointer to it in diff_manifold.rst. To generate the documentation in a fast way, i.e. without having Sage process the whole reference manual, run

sage -docbuild reference/manifolds html


### comment:42 Changed 3 years ago by egourgoulhon

Side remark: the bug that you've reported in https://groups.google.com/d/msg/sage-devel/EfLYpAxl_jU/ShlCS9L4BgAJ is fixed in Sage 8.8.beta1 (since #25576 has been merged in it).

### comment:43 in reply to: ↑ 41 Changed 3 years ago by gh-DeRhamSource

Regarding the documentation: you have added doctests in the Python sources but the documentation is not generated since there is no entry to it in the sources of the reference manual. You shall modify/add files in src/doc/en/reference/manifolds, for instance in a way similar to what is done for differential forms, i.e. create a file mixed_diff_form.rst and add a pointer to it in diff_manifold.rst. To generate the documentation in a fast way, i.e. without having Sage process the whole reference manual, run

sage -docbuild reference/manifolds html


Unfortunately, I get some errors:

[manifolds] /home/michi/GitProjects/sage/src/sage_setup/docbuild/ext/sage_autodoc.py:1170: RemovedInSphinx20Warning: formatargspec() is now deprecated.  Please use sphinx.util.inspect.Signature instead.
[manifolds]   return formatargspec(initmeth, *argspec)
[manifolds]   args = formatargspec(self.object, *argspec)
[manifolds]   args = formatargspec(self.object, *argspec)
...
OSError: /home/michi/GitProjects/sage/src/doc/en/reference/manifolds/index.rst:4: WARNING: undefined label: tensors-on-free-modules (if the link has no caption the label must precede a section header)


However, what comes next?

Last edited 3 years ago by gh-DeRhamSource (previous) (diff)

### comment:44 Changed 3 years ago by git

• Commit changed from 506ef37791cdd89b1b2fa9b8009827197205c17e to 970ccce6b788861b77e8cb9b6502f74bd369ab1d

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

 ​970ccce Minor fixes

### comment:45 Changed 3 years ago by git

• Commit changed from 970ccce6b788861b77e8cb9b6502f74bd369ab1d to 9516d69ee612b353ff4e4a60f5b02c14601ff61b

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

 ​9516d69 Sorry!

### comment:46 Changed 3 years ago by git

• Commit changed from 9516d69ee612b353ff4e4a60f5b02c14601ff61b to 3a731b678258d93c13d1ba3190afcb8def3a69a6

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

 ​3a731b6 Damn! :D

### comment:47 Changed 3 years ago by tscrim

Some debugging will be needed (if you haven't fixed them already). Also you should rebase this (and force push) to have more professional commit messages. ;)

### comment:48 follow-up: ↓ 49 Changed 3 years ago by gh-DeRhamSource

What should be debugged? I mean, the error above does not seem to have to do something with my implementation. Or does it?

Version 0, edited 3 years ago by gh-DeRhamSource (next)

### comment:49 in reply to: ↑ 48 Changed 3 years ago by egourgoulhon

What should be debugged? I mean, the error above does not seem to have to do something with my implementation. Or does it?

Long story short: There is no error occuring anymore regarding the mixed algebra while building the doc.

Yes but some parts of the documentation require some debugging:

• a formula is not displayed because .. MATH: should be .. MATH:: in line 45 of mixed_form.py
• in line 60 of the same file, there is an extra backquote around comp
• at the top of mixed_form.py you may want to replace \Phi by \varphi to be consistent with the rest of the text, as well as with mixed_form_algebra.py

### comment:50 Changed 3 years ago by git

• Commit changed from 3a731b678258d93c13d1ba3190afcb8def3a69a6 to 177cfff96044aa16d00a95ffcc7798f63cf29ec3

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

 ​177cfff Minor fixes

### comment:51 Changed 3 years ago by git

• Commit changed from 177cfff96044aa16d00a95ffcc7798f63cf29ec3 to 4d9967aa4640d600131d17c57d4683c8568bbcc5

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

 ​38b36ed Minor fixes ​4d9967a Minor doc fixes

### comment:52 Changed 3 years ago by gh-DeRhamSource

My apologies for these confusing commits. I'm quite new to it.

However, I fixed the aforementioned issues.

Last edited 3 years ago by gh-DeRhamSource (previous) (diff)

### comment:53 Changed 3 years ago by tscrim

I have a couple more comments, most of them are fairly minor:

In _richcmp_:

        from sage.structure.richcmp import richcmp

# Compare all elements separately:
for j in range(0, self._max_deg + 1):


move the import statement to the top-level (this is for speed reasons) and make range(0, foo) -> range(foo).

Make _mul_ = wedge and maybe combine or reduce the doctests.

degree $k$ along \varphi replace & with .

Instead of using self.__class__, I have been told it is better to use type(self).

Instead of \, my understanding is it is better to use () as that is guaranteed to generate an error if misused (as opposed to possibly a strange bug). For example

-                error_msg = "input must be a " \
-                            "differential form of degree {}".format(deg)
+                error_msg = ("input must be a "
+                             "differential form of degree {}".format(deg))


Typo: datailed -> detailed

I don't think you need the self._populate_coercion_lists_().

### comment:54 Changed 3 years ago by git

• Commit changed from 4d9967aa4640d600131d17c57d4683c8568bbcc5 to cebde90d71134ea8c83ab3e96028cf123a4b7ad3

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

 ​cebde90 Typos and Minor Issues Fixed

### comment:55 Changed 3 years ago by git

• Commit changed from cebde90d71134ea8c83ab3e96028cf123a4b7ad3 to b78c7a9f8a928250b60278eb510d4f041d540849

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

 ​b78c7a9 Whitespace Correction

### comment:56 Changed 3 years ago by git

• Commit changed from b78c7a9f8a928250b60278eb510d4f041d540849 to 407505d3827641a010caf3fcd11bed4df88b78b3

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

 ​407505d Components from scratch

### comment:57 Changed 3 years ago by git

• Commit changed from 407505d3827641a010caf3fcd11bed4df88b78b3 to f03d4a0f22df2c6abcb41f3afa5b74bd392f3cf1

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

 ​f03d4a0 Minor: Whitespace + Comment

### comment:58 Changed 3 years ago by git

• Commit changed from f03d4a0f22df2c6abcb41f3afa5b74bd392f3cf1 to b4a92196526d32e171c896e59e90a90d46865d6e

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

 ​b4a9219 Error Message obsolete

### comment:59 Changed 3 years ago by gh-DeRhamSource

Thanks for your patience so far. Again, I changed a lot. Please check.

However, I noticed another error message that should not occur:

sage: M = Manifold(2, 'M')
sage: X.<x,y> = M.chart()
sage: f = M.scalar_field(x, name='f')
sage: f in M.diff_form_module(1)
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-38-9654b754d2bc> in <module>()
2 X = M.chart(names=('x', 'y',)); (x, y,) = X._first_ngens(2)
3 f = M.scalar_field(x, name='f')
----> 4 f in M.diff_form_module(Integer(1))

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/structure/parent.pyx in sage.structure.parent.Parent.__contains__ (build/cythonized/sage/structure/parent.c:9885)()
1090             return True
1091         try:
-> 1092             x2 = self(x)
1093             EQ = (x2 == x)
1094             if EQ is True:

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/structure/parent.pyx in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9219)()
898         if mor is not None:
899             if no_extra_args:
--> 900                 return mor._call_(x)
901             else:
902                 return mor._call_with_args(x, args, kwds)

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/structure/coerce_maps.pyx in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4551)()
160                 print(type(C), C)
161                 print(type(C._element_constructor), C._element_constructor)
--> 162             raise
163
164     cpdef Element _call_with_args(self, x, args=(), kwds={}):

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/structure/coerce_maps.pyx in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4443)()
155         cdef Parent C = self._codomain
156         try:
--> 157             return C._element_constructor(x)
158         except Exception:
159             if print_warnings:

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/manifolds/differentiable/diff_form_module.pyc in _element_constructor_(self, comp, frame, name, latex_name)
818         resu = self.element_class(self._fmodule, self._degree, name=name,
819                                   latex_name=latex_name)
--> 820         if comp != []:
821             resu.set_comp(frame)[:] = comp
822         return resu

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/manifolds/scalarfield.pyc in __ne__(self, other)
1285
1286         """
-> 1287         return not (self == other)
1288
1289     ####### End of required methods for an algebra element (beside arithmetic) #######

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/manifolds/scalarfield.pyc in __eq__(self, other)
1244                 return False
1245             try:
-> 1246                 other = self.parent()(other)  # conversion to a scalar field
1247             except TypeError:
1248                 return False

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/structure/parent.pyx in sage.structure.parent.Parent.__call__ (build/cythonized/sage/structure/parent.c:9219)()
898         if mor is not None:
899             if no_extra_args:
--> 900                 return mor._call_(x)
901             else:
902                 return mor._call_with_args(x, args, kwds)

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/structure/coerce_maps.pyx in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4551)()
160                 print(type(C), C)
161                 print(type(C._element_constructor), C._element_constructor)
--> 162             raise
163
164     cpdef Element _call_with_args(self, x, args=(), kwds={}):

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/structure/coerce_maps.pyx in sage.structure.coerce_maps.DefaultConvertMap_unique._call_ (build/cythonized/sage/structure/coerce_maps.c:4443)()
155         cdef Parent C = self._codomain
156         try:
--> 157             return C._element_constructor(x)
158         except Exception:
159             if print_warnings:

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/manifolds/scalarfield_algebra.pyc in _element_constructor_(self, coord_expression, chart, name, latex_name)
470                                       coord_expression=coord_expression,
471                                       name=name, latex_name=latex_name,
--> 472                                       chart=chart)
473         return resu
474

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/manifolds/differentiable/scalarfield.pyc in __init__(self, parent, coord_expression, chart, name, latex_name)
630         """
631         ScalarField.__init__(self, parent, coord_expression=coord_expression,
--> 632                              chart=chart, name=name, latex_name=latex_name)
633         self._tensor_type = (0,0)
634

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/manifolds/scalarfield.pyc in __init__(self, parent, coord_expression, chart, name, latex_name)
1101                         self._express[ch] = ch.function(coord_expression)
1102                 else:
-> 1103                     self._express[chart] = chart.function(coord_expression)
1104         self._init_derived()   # initialization of derived quantities
1105

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/manifolds/chart.pyc in function(self, expression, calc_method, expansion_symbol, order)
1087         return parent.element_class(parent, expression, calc_method=calc_method,
1088                                     expansion_symbol=expansion_symbol,
-> 1089                                     order=order)
1090
1091     def zero_function(self):

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/manifolds/chart_func.pyc in __init__(self, parent, expression, calc_method, expansion_symbol, order)
360                 calc_method = self._calc_method._current
361             self._express[calc_method] = self._calc_method._tranf[calc_method](
--> 362                                                                     expression)
363         # Derived quantities:
364         self._der = None  # list of partial derivatives (to be set by diff()

/home/michi/GitProjects/sage/local/lib/python2.7/site-packages/sage/manifolds/calculus_method.pyc in _Sympy_to_SR(expression)
106         # If SR cannot transform a sympy expression this is because it is a
107         # sympy abstract function
--> 108         a = expression._sage_()
109         # As all sage objects have a ._sage_ operator, they have to be
110         # catched

AttributeError: 'list' object has no attribute '_sage_'


### comment:60 Changed 3 years ago by git

• Commit changed from b4a92196526d32e171c896e59e90a90d46865d6e to 8036fb0b38fb4add88e5c54b1c1d071074fbc9b8

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

 ​8036fb0 Components from scratch - Fixes

### comment:61 Changed 3 years ago by git

• Commit changed from 8036fb0b38fb4add88e5c54b1c1d071074fbc9b8 to 6d1c88cb09a31613dd921d2bef7c71cfbfd27ed6

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

 ​6d1c88c Tiny doc typos fixed

### comment:62 follow-up: ↓ 63 Changed 3 years ago by tscrim

Does that second to last commit fix the issue in comment:59?

### comment:63 in reply to: ↑ 62 ; follow-up: ↓ 64 Changed 3 years ago by gh-DeRhamSource

Does that second to last commit fix the issue in comment:59?

No. But now, I guess this has nothing to do with the mixed algebra. I already reported it in sage-devel.

Last edited 3 years ago by gh-DeRhamSource (previous) (diff)

### comment:64 in reply to: ↑ 63 ; follow-up: ↓ 65 Changed 3 years ago by egourgoulhon

Does that second to last commit fix the issue in comment:59?

No. But now, I guess this has nothing to do with the mixed algebra. I already reported it in sage-devel.

I confirm the error. I've opened #27658 for it.

### comment:65 in reply to: ↑ 64 Changed 3 years ago by egourgoulhon

Does that second to last commit fix the issue in comment:59?

No. But now, I guess this has nothing to do with the mixed algebra. I already reported it in sage-devel.

I confirm the error. I've opened #27658 for it.

While this is definitely a bug, you can get rid of it by replacing

f in M.diff_form_module(n)


by

f.domain() is M and f.degree() == n


for any integer n. Moreover the second form is must faster.

Last edited 3 years ago by egourgoulhon (previous) (diff)

### comment:66 follow-up: ↓ 67 Changed 3 years ago by gh-DeRhamSource

Fortunately, this issue is not directly relevant for my code anymore (since the error message was deleted). I've encountered the problem while debugging. :)

Last edited 3 years ago by gh-DeRhamSource (previous) (diff)

### comment:67 in reply to: ↑ 66 Changed 3 years ago by egourgoulhon

Fortunately, this issue is not directly relevant for my code anymore (since the error message was deleted). I've encountered the problem while debugging. :)

Good!

### comment:68 Changed 3 years ago by gh-DeRhamSource

Patchbot says no. :(

Though, there is no obvious error. So, what's the matter?

Any further suggestions?

### comment:69 follow-up: ↓ 70 Changed 3 years ago by tscrim

That error from the patchbot looks unrelated, so I would treat the patchbot as coming back green.

Sorry, one more nitpick for readability:

-            return self._domain.is_subset(
-                S._domain) and self._ambient_domain.is_subset(S._ambient_domain)
+            return (self._domain.is_subset(S._domain)
+                    and self._ambient_domain.is_subset(S._ambient_domain))


### comment:70 in reply to: ↑ 69 Changed 3 years ago by egourgoulhon

Yes, there is a doctest error with Python3-built Sage:

sage -t --long --warn-long 48.2 src/sage/manifolds/differentiable/mixed_form.py
**********************************************************************
File "src/sage/manifolds/differentiable/mixed_form.py", line 479, in sage.manifolds.differentiable.mixed_form.MixedForm.set_name
Failed example:
F.set_name(name='fancy', latex_name='\eta'); F
Expected:
Mixed differential form fancy on the 4-dimensional differentiable
manifold M
Got:
doctest:warning
...
DeprecationWarning: invalid escape sequence \e
Mixed differential form fancy on the 4-dimensional differentiable manifold M
**********************************************************************


You should replace '\eta' by r'\eta'.

Apart from this, everything is OK for me.

Last edited 3 years ago by egourgoulhon (previous) (diff)

### comment:71 Changed 3 years ago by git

• Commit changed from 6d1c88cb09a31613dd921d2bef7c71cfbfd27ed6 to cdfaf6d16322ef4c7d014680ebf025f272f62eb6

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

 ​cdfaf6d Doctest fix + Readability`

### comment:72 follow-up: ↓ 73 Changed 3 years ago by gh-DeRhamSource

Anything else? :)

Otherwise I'd start programming the next part - namely characteristic classes.

### comment:73 in reply to: ↑ 72 Changed 3 years ago by egourgoulhon

Anything else? :)

Otherwise I'd start programming the next part - namely characteristic classes.

Thanks for the update and your work on this. I am OK for the positive review. Travis, do you agree?

### comment:74 Changed 3 years ago by tscrim

• Status changed from needs_review to positive_review

I concur.

### comment:75 Changed 3 years ago by vbraun

• Branch changed from public/manifolds/mixed_differential_forms to cdfaf6d16322ef4c7d014680ebf025f272f62eb6
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.