Opened 3 years ago
Closed 3 years ago
#27584 closed enhancement (fixed)
Characteristic Classes  MixedAlgebra
Reported by:  ghDeRhamSource  Owned by:  

Priority:  major  Milestone:  sage8.8 
Component:  geometry  Keywords:  characteristic classes, manifolds 
Cc:  egourgoulhon, tscrim  Merged in:  
Authors:  Michael Jung  Reviewers:  Eric Gourgoulhon, Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  cdfaf6d (Commits, GitHub, GitLab)  Commit:  cdfaf6d16322ef4c7d014680ebf025f272f62eb6 
Dependencies:  Stopgaps: 
Description (last modified by )
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
Change History (75)
comment:1 Changed 3 years ago by
 Cc egourgoulhon added
 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
 Cc tscrim added
 Description modified (diff)
 Summary changed from Characteristic Classes to Characteristic Classes  MixedAlgebra
comment:3 Changed 3 years ago by
 Branch set to u/DeRhamSource/char_class_algebra
comment:4 Changed 3 years ago by
 Branch changed from u/DeRhamSource/char_class_algebra to u/ghDeRhamSource/char_class_algebra
comment:5 Changed 3 years ago by
 Branch u/ghDeRhamSource/char_class_algebra deleted
comment:6 Changed 3 years ago by
 Branch set to u/ghDeRhamSource/characteristic_classes___mixedalgebra
comment:7 Changed 3 years ago by
 Commit set to 49f21e50d792f4000c0b0b6cf64c93fd5eaf2da2
comment:8 Changed 3 years ago by
Here is the answer to the question raised in https://groups.google.com/d/msg/sagedevel/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 2dimensional 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 22692271 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
 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
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
The other thing you could do is add the notebook file as an attachment (but note that you cannot delete it afterwards).
comment:12 followup: ↓ 13 Changed 3 years ago by
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.
comment:13 in reply to: ↑ 12 Changed 3 years ago by
Replying to ghDeRhamSource:
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
 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
 Description modified (diff)
comment:16 Changed 3 years ago by
 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
I added a link to my public github repo for the demo notebook in the description.
comment:18 Changed 3 years ago by
 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
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) <ipythoninput847bd889258762> 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
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.
comment:21 Changed 3 years ago by
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 2dimensional differentiable manifold M
comment:22 Changed 3 years ago by
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
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 2dimensional differentiable manifold M Arithmetic performed after coercions. Result lives in Graded algebra Omega^*(M) of mixed differential forms on the 2dimensional differentiable manifold M
comment:24 Changed 3 years ago by
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
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 2dimensional 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
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
I'm sorry: How do I manage that?
comment:28 Changed 3 years ago by
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 followups: ↓ 31 ↓ 35 Changed 3 years ago by
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 2dimensional 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
Yes, perfect. That solved the problem. Thanks! :)
comment:31 in reply to: ↑ 29 ; followup: ↓ 32 Changed 3 years ago by
Replying to egourgoulhon:
To make it work, you should implement the method
_lmul_
in classMixedForm
and add the lineif other is SR: return Truein
MixedFormAlgebra._coerce_map_from_
. At least, this is what is done for the algebras of scalar fields: see line 512 ofsrc/sage/manifolds/scalarfield_algebra.py
and the definition of_lmul_
in line 2449 ofsrc/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 ; followup: ↓ 36 Changed 3 years ago by
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_
?
comment:33 Changed 3 years ago by
 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
 Status changed from new to needs_review
comment:35 in reply to: ↑ 29 ; followup: ↓ 38 Changed 3 years ago by
Replying to egourgoulhon:
To make it work, you should implement the method
_lmul_
in classMixedForm
and add the lineif other is SR: return Truein
MixedFormAlgebra._coerce_map_from_
. At least, this is what is done for the algebras of scalar fields: see line 512 ofsrc/sage/manifolds/scalarfield_algebra.py
and the definition of_lmul_
in line 2449 ofsrc/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 ; followup: ↓ 37 Changed 3 years ago by
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)
whenself
andother
have the same parent_lmul_(self, other)
whenself
andother
have distinct parents_rmul_(self, other)
is inherited fromModuleElement
(if not redefined) and falls back to_lmul_
Travis, do you confirm?
comment:37 in reply to: ↑ 36 Changed 3 years ago by
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 theModuleElement
level (actually returnsNone
). 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)
whenself
andother
have the same parent_lmul_(self, other)
whenself
andother
have distinct parents_rmul_(self, other)
is inherited fromModuleElement
(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
Replying to ghDeRhamSource:
Replying to egourgoulhon:
To make it work, you should implement the method
_lmul_
in classMixedForm
and add the lineif other is SR: return Truein
MixedFormAlgebra._coerce_map_from_
. At least, this is what is done for the algebras of scalar fields: see line 512 ofsrc/sage/manifolds/scalarfield_algebra.py
and the definition of_lmul_
in line 2449 ofsrc/sage/manifolds/scalarfield.py
.I managed the coercion detection via
if self._domain.scalar_field_algebra().has_coerce_map_from(S): return TrueDoes 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
 Branch changed from u/ghDeRhamSource/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/ghDeRhamSource/characteristic_classes___mixedalgebra' of git://trac.sagemath.org/sage into Sage 8.8.beta1

comment:40 Changed 3 years ago by
 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/computationalmathematicswithsagemath/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 followup: ↓ 43 Changed 3 years ago by
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
Side remark: the bug that you've reported in https://groups.google.com/d/msg/sagedevel/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
Replying to 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 filemixed_diff_form.rst
and add a pointer to it indiff_manifold.rst
. To generate the documentation in a fast way, i.e. without having Sage process the whole reference manual, runsage 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] /home/michi/GitProjects/sage/src/sage_setup/docbuild/ext/sage_autodoc.py:1406: RemovedInSphinx20Warning: formatargspec() is now deprecated. Please use sphinx.util.inspect.Signature instead. [manifolds] args = formatargspec(self.object, *argspec) [manifolds] /home/michi/GitProjects/sage/src/sage_setup/docbuild/ext/sage_autodoc.py:1072: RemovedInSphinx20Warning: formatargspec() is now deprecated. Please use sphinx.util.inspect.Signature instead. [manifolds] args = formatargspec(self.object, *argspec) ... OSError: /home/michi/GitProjects/sage/src/doc/en/reference/manifolds/index.rst:4: WARNING: undefined label: tensorsonfreemodules (if the link has no caption the label must precede a section header)
However, what comes next?
comment:44 Changed 3 years ago by
 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
 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
 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
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 followup: ↓ 49 Changed 3 years ago by
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.
comment:49 in reply to: ↑ 48 Changed 3 years ago by
Replying to ghDeRhamSource:
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 ofmixed_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 withmixed_form_algebra.py
comment:50 Changed 3 years ago by
 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
 Commit changed from 177cfff96044aa16d00a95ffcc7798f63cf29ec3 to 4d9967aa4640d600131d17c57d4683c8568bbcc5
comment:52 Changed 3 years ago by
My apologies for these confusing commits. I'm quite new to it.
However, I fixed the aforementioned issues.
comment:53 Changed 3 years ago by
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 toplevel (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
 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
 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
 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
 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
 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
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) <ipythoninput389654b754d2bc> 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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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
 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
 Commit changed from 8036fb0b38fb4add88e5c54b1c1d071074fbc9b8 to 6d1c88cb09a31613dd921d2bef7c71cfbfd27ed6
Branch pushed to git repo; I updated commit sha1. New commits:
6d1c88c  Tiny doc typos fixed

comment:62 followup: ↓ 63 Changed 3 years ago by
Does that second to last commit fix the issue in comment:59?
comment:63 in reply to: ↑ 62 ; followup: ↓ 64 Changed 3 years ago by
Replying to tscrim:
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 sagedevel.
comment:64 in reply to: ↑ 63 ; followup: ↓ 65 Changed 3 years ago by
Replying to ghDeRhamSource:
Replying to tscrim:
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 sagedevel.
I confirm the error. I've opened #27658 for it.
comment:65 in reply to: ↑ 64 Changed 3 years ago by
Replying to egourgoulhon:
Replying to ghDeRhamSource:
Replying to tscrim:
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 sagedevel.
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.
comment:66 followup: ↓ 67 Changed 3 years ago by
Fortunately, this issue is not directly relevant for my code anymore (since the error message was deleted). I've encountered the problem while debugging. :)
comment:67 in reply to: ↑ 66 Changed 3 years ago by
Replying to ghDeRhamSource:
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
Patchbot says no. :(
Though, there is no obvious error. So, what's the matter?
Any further suggestions?
comment:69 followup: ↓ 70 Changed 3 years ago by
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))
Eric, any other comments?
comment:70 in reply to: ↑ 69 Changed 3 years ago by
Replying to tscrim:
Eric, any other comments?
Yes, there is a doctest error with Python3built Sage:
sage t long warnlong 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 4dimensional differentiable manifold M Got: doctest:warning ... DeprecationWarning: invalid escape sequence \e Mixed differential form fancy on the 4dimensional differentiable manifold M **********************************************************************
You should replace '\eta'
by r'\eta'
.
Apart from this, everything is OK for me.
comment:71 Changed 3 years ago by
 Commit changed from 6d1c88cb09a31613dd921d2bef7c71cfbfd27ed6 to cdfaf6d16322ef4c7d014680ebf025f272f62eb6
Branch pushed to git repo; I updated commit sha1. New commits:
cdfaf6d  Doctest fix + Readability

comment:72 followup: ↓ 73 Changed 3 years ago by
Anything else? :)
Otherwise I'd start programming the next part  namely characteristic classes.
comment:73 in reply to: ↑ 72 Changed 3 years ago by
Replying to ghDeRhamSource:
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:75 Changed 3 years ago by
 Branch changed from public/manifolds/mixed_differential_forms to cdfaf6d16322ef4c7d014680ebf025f272f62eb6
 Resolution set to fixed
 Status changed from positive_review to closed
Branch pushed to git repo; I updated commit sha1. New commits:
Initial version (without doctests)