Opened 2 years ago
Closed 10 months ago
#29581 closed enhancement (fixed)
New Algorithm for Characteristic Classes
Reported by: | gh-mjungmath | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.5 |
Component: | manifolds | Keywords: | characteristic_classes, manifolds |
Cc: | slelievre, egourgoulhon, tscrim, mkoeppe, gh-tobiasdiez | Merged in: | |
Authors: | Michael Jung | Reviewers: | Travis Scrimshaw, Eric Gourgoulhon |
Report Upstream: | N/A | Work issues: | |
Branch: | ac5cbbf (Commits, GitHub, GitLab) | Commit: | ac5cbbf0072380022c32466aab95fdfb31c4ecee |
Dependencies: | #32270, #32272, #32396 | Stopgaps: |
Description (last modified by )
The current algorithm for characteristic forms is comparably slow. The worst case scenario showed computations times about 1h in dimension four.
With this ticket, we want to replace the current algorithm and implement characteristic cohomology classes as sub-ring of the de Rham cohomology ring (cf. #31691).
The idea is that characteristic (cohomology) classes are generated by Chern/Pontryagin/Euler? classes. The generators can be computed by an Faddeev-LeVerrier?-like algorithm (cf. #30681).
Change History (126)
comment:1 Changed 2 years ago by
- Branch set to u/gh-mjungmath/new_algorithm
comment:2 Changed 2 years ago by
- Commit set to 1a803eba5b9724ebfa93e726a474d957fd6915b4
- Milestone changed from sage-9.1 to sage-9.2
comment:3 Changed 2 years ago by
- Cc slelievre added
- Component changed from PLEASE CHANGE to geometry
comment:4 Changed 2 years ago by
Please add a short description in the "Description" field of the ticket,
and the author's full name in the "Authors" field of the ticket. Don't forget
to set to needs_review
when this is ready for review.
comment:5 Changed 2 years ago by
- Commit changed from 1a803eba5b9724ebfa93e726a474d957fd6915b4 to cb442eb8919ca33a5db63a17bac52f99f55553dd
Branch pushed to git repo; I updated commit sha1. New commits:
730046c | Merge branch 'develop' into t/29570/diff_form_bug
|
130ae3f | Trac #29570: NotImplementedError for non-generic ring elements
|
bba21e1 | Trac #29570: Strange typo fixed...
|
ba3b4b9 | Trac #29570: correct parent, vectorfield_module changes reverted
|
841e1bf | Merge branch 't/29570/diff_form_bug' into t/29581/new_algorithm
|
cb442eb | Trac #29570: new algorithm added and code cleaned
|
comment:6 Changed 2 years ago by
- Commit changed from cb442eb8919ca33a5db63a17bac52f99f55553dd to 15c7341d071327a04fd229c3d2f78d583564f476
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
15c7341 | Trac #29581: new algorithm added and code cleaned
|
comment:7 Changed 2 years ago by
- Cc egourgoulhon added
- Description modified (diff)
- Keywords characteristic_classes manifolds added
- Type changed from PLEASE CHANGE to enhancement
comment:8 Changed 2 years ago by
Correct syntax for crosslinks is
.. SEEALSO::
comment:9 Changed 2 years ago by
- do not use this:
if distinct_real == False
but
if not distinct_real
(at least in two places)
- The following change is wrong:
- - ``cmatrix`` -- curvature matrix - + - ``cmatrix`` -- curvature matrix OUTPUT: - - ``I/(2*pi)*cmatrix`` + - ``I/(2*pi)*cmatrix``
as one should not indent inside INPUT or OUTPUT blocks (because they end with only one colon). And moreover, one does want empty lines to separate these things.
This is also incorrect:
- self._dual_exterior_powers[p] = ExtPowerDualFreeModule(self, p) + self._dual_exterior_powers[p] = ExtPowerDualFreeModule(self, p)
Why did you change the indentation ? it was ok.
comment:10 Changed 2 years ago by
I am sorry. At this stage, this is just a draft. The indentations are typos coming from using alt+tab (at least I guess so). I will fix this soon.
comment:11 Changed 2 years ago by
Thanks for your advice. :)
comment:12 Changed 2 years ago by
- Commit changed from 15c7341d071327a04fd229c3d2f78d583564f476 to d37229341effff46ea2fa4b3287e32997c8422b0
Branch pushed to git repo; I updated commit sha1. New commits:
d372293 | Trac #29581: Strange typos reverted
|
comment:13 Changed 2 years ago by
- Status changed from new to needs_review
comment:14 Changed 2 years ago by
- Status changed from needs_review to needs_info
comment:15 Changed 2 years ago by
An example for the application of the new algorithm would be really nice. For instance a Todd class or A-Hat class. But I have no idea for a suitable one. If anyone does, or at least knows someone who does, I would really appreciate it. Thanks! :)
comment:16 Changed 2 years ago by
- Commit changed from d37229341effff46ea2fa4b3287e32997c8422b0 to 19815c4257cee7dcf503d94e3b9bb651150bef5f
comment:17 Changed 2 years ago by
- Component changed from geometry to manifolds
- Dependencies set to #30211
comment:18 Changed 22 months ago by
- Milestone changed from sage-9.2 to sage-9.3
comment:19 Changed 17 months ago by
- Milestone changed from sage-9.3 to sage-9.4
Setting new milestone based on a cursory review of ticket status, priority, and last modification date.
comment:20 Changed 13 months ago by
- Milestone changed from sage-9.4 to sage-9.5
Setting a new milestone for this ticket based on a cursory review.
comment:21 Changed 13 months ago by
- Cc tscrim mkoeppe added
- Dependencies #30211 deleted
- Description modified (diff)
- Status changed from needs_info to needs_work
comment:22 Changed 13 months ago by
- Commit changed from 19815c4257cee7dcf503d94e3b9bb651150bef5f to 3c9b80a2551994493f23640344e9ade63240cb01
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
3c9b80a | Trac #29581: implement algorithms for chern, pontryagin and euler
|
comment:23 Changed 13 months ago by
Do you think it is necessary to give those cohomology classes an own parent or would DeRhamCohomologyRing
as parent suffice?
The point is that addition and multiplication is performed differently among those elements. Moreover, it is possible to compare them.
comment:24 Changed 13 months ago by
- Description modified (diff)
comment:25 Changed 13 months ago by
- Status changed from needs_work to needs_info
comment:26 Changed 13 months ago by
- Description modified (diff)
comment:27 Changed 13 months ago by
- Commit changed from 3c9b80a2551994493f23640344e9ade63240cb01 to 03529f8919357f98b65ee033e8dc5c7d48b2fa42
Branch pushed to git repo; I updated commit sha1. New commits:
03529f8 | Trac #29581: return mixed form
|
comment:28 Changed 12 months ago by
- Commit changed from 03529f8919357f98b65ee033e8dc5c7d48b2fa42 to ab288d62786a40b9f314d40ca932506d7b6932d9
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
fb9c661 | #32270: de rham cohomology as algebra
|
de5863a | Merge branch 't/32270/turn_de_rham_cohomology_into_algebra' into t/29581/new_algorithm
|
d22ee9a | Trac #32315: support iteration and enumerated sets
|
16c4737 | Trac #32272: allow infinite max degree
|
6a320a0 | Trac #32272: fix docstring
|
c1d4fce | Trac #32272: fix rst file + rename module
|
25e6b1d | Trac #32272: fix doctest
|
5a4c7ac | Trac #32272: narrow to finite dimensions
|
3b81085 | Merge branch 't/32272/finitely_generated_graded_algebras_with_finite_degree' into t/29581/new_algorithm
|
ab288d6 | Trac #29581: algorithm for complex vbundles
|
comment:29 Changed 12 months ago by
- Dependencies set to #32270, #32272
comment:30 Changed 12 months ago by
- Commit changed from ab288d62786a40b9f314d40ca932506d7b6932d9 to 6688740017097a79d7995101889cea097ee5c653
comment:31 Changed 12 months ago by
- Commit changed from 6688740017097a79d7995101889cea097ee5c653 to 61291f406b886abb9a509955355da749fc99a8d6
Branch pushed to git repo; I updated commit sha1. New commits:
61291f4 | Trac #29581: minor fixes
|
comment:32 Changed 12 months ago by
- Commit changed from 61291f406b886abb9a509955355da749fc99a8d6 to 60318775e9c530c32280ba1bfa819cb9ac978bd1
Branch pushed to git repo; I updated commit sha1. New commits:
6031877 | Trac #29581: return only list of generators
|
comment:33 Changed 12 months ago by
- Commit changed from 60318775e9c530c32280ba1bfa819cb9ac978bd1 to 0c7587f759b9555404fd417ee042e99fbb4ae02d
Branch pushed to git repo; I updated commit sha1. New commits:
0c7587f | Trac #29581: parent deals with different cases, element stays generic
|
comment:34 Changed 12 months ago by
- Commit changed from 0c7587f759b9555404fd417ee042e99fbb4ae02d to 28d2d84271a3e1516556a8364b1114dd6edaa992
Branch pushed to git repo; I updated commit sha1. New commits:
28d2d84 | Trac #29581: revert mistaken merge
|
comment:35 Changed 12 months ago by
The ticket is not ready for review yet; the structure should be fixed though. Feedback at this stage is very much appreciated. :)
comment:36 Changed 12 months ago by
- Commit changed from 28d2d84271a3e1516556a8364b1114dd6edaa992 to 2cabd839d55e5c0f3ba2f8d0194bcae6ce31397c
Branch pushed to git repo; I updated commit sha1. New commits:
2cabd83 | Trac #29581: add some comments
|
comment:37 Changed 12 months ago by
- Dependencies changed from #32270, #32272 to #32270, #32272, #32396
comment:38 Changed 12 months ago by
- Description modified (diff)
comment:39 Changed 12 months ago by
- Commit changed from 2cabd839d55e5c0f3ba2f8d0194bcae6ce31397c to 815392c4bcf2a6177cecbcc24ae3dc73372fe743
Branch pushed to git repo; I updated commit sha1. New commits:
1a9b7d9 | Trac #29581: compute Euler forms for arbitrary positively oriented frames
|
fcd64c9 | Trac #32396: add __abs__ to chart_func and scalarfield
|
6d70be4 | Merge branch 'abs_function_for_scalar_fields' into t/29581/new_algorithm
|
815392c | Trac #29581: fix some bugs + treat Euler case
|
comment:40 Changed 12 months ago by
- Commit changed from 815392c4bcf2a6177cecbcc24ae3dc73372fe743 to 62ff48af88ee24979df68297551a540a47dbc1a3
Branch pushed to git repo; I updated commit sha1. New commits:
62ff48a | Trac #29581: leave comment on to-do
|
comment:41 Changed 12 months ago by
- Commit changed from 62ff48af88ee24979df68297551a540a47dbc1a3 to 5b8632d7b5ac52bfe943660b26ffabe35b2c7a8f
Branch pushed to git repo; I updated commit sha1. New commits:
5b8632d | Trac #29581: euler form algorithm finished
|
comment:42 Changed 12 months ago by
- Commit changed from 5b8632d7b5ac52bfe943660b26ffabe35b2c7a8f to 4afc9f0d0ce1212516c67f6d04617151ef4d0ea3
Branch pushed to git repo; I updated commit sha1. New commits:
4afc9f0 | Trac #29581: compute forms only on demand
|
comment:43 follow-up: ↓ 46 Changed 12 months ago by
I am particularly curious what you think about my last commit. I want that the corresponding characteristic forms are only computed if really necessary. This essentially only matters for the Pontryagin-Euler algorithm when the characteristic cohomology class contains no Euler or Pontryagin part.
Is there any lazy-stuff to make this a bit nicer maybe?
comment:44 Changed 12 months ago by
- Commit changed from 4afc9f0d0ce1212516c67f6d04617151ef4d0ea3 to 201831f1338973a6c5eefb6c9ee61592f9e38f0f
Branch pushed to git repo; I updated commit sha1. New commits:
201831f | Trac #29581: commenting improved
|
comment:45 Changed 12 months ago by
Since I have implemented fast_wedge_power
, which can be used separately of course, do we want a power method for differential forms or tensor fields respectively?
comment:46 in reply to: ↑ 43 Changed 12 months ago by
Replying to gh-mjungmath:
I am particularly curious what you think about my last commit. I want that the corresponding characteristic forms are only computed if really necessary. This essentially only matters for the Pontryagin-Euler algorithm when the characteristic cohomology class contains no Euler or Pontryagin part.
You would have to know that ahead of time I think. Naïvely, it seems like you need to know the answer before you can tell if the answer is the form you want.
comment:47 Changed 12 months ago by
Well, what I need to know is already encoded in the characteristic cohomology class itself. For example, if it contains no Euler class generator, then I don't need to invoke EulerAlgorithm().get(nab)
. Like I said, this only matters for the "mixed" PontryaginEuler? case because in any other case I have to do the full computation anyway.
One reason why this is important: in the odd-rank case, or for non-Levi-Civita connections, this algorithm for the Euler class is not working. But one may still want to compute the Pontryagin classes.
The current commit makes sure of that by asking for the power of a certain generator and returning the one scalar field right away if the exponent is zero. This works, but it feels rather dirty to me, and I ponder whether there might be a nicer solution to this.
comment:48 Changed 12 months ago by
- Commit changed from 201831f1338973a6c5eefb6c9ee61592f9e38f0f to e17fe37d0ea5ecdd1e9932145b908037ed0261cd
Branch pushed to git repo; I updated commit sha1. New commits:
e17fe37 | Trac #29581: comments further improved + _repr_ method
|
comment:49 follow-up: ↓ 50 Changed 12 months ago by
Unfortunately this is over my head mathematically, so I don't think I can really help with possible solutions at this point.
comment:50 in reply to: ↑ 49 ; follow-up: ↓ 52 Changed 12 months ago by
Replying to tscrim:
Unfortunately this is over my head mathematically, so I don't think I can really help with possible solutions at this point.
Mh okay. If you like, we can talk about it privately at some point.
I have another question though. I tried to establish a coercion from the characteristic cohomology class ring to the de Rham cohomology ring. For that, I need a representative which I only get if I've inserted a connection. Now I've read that coercions must always suceed. So throwing an error if no representative is available is not an option then?
Otherwise, we could return the same instance of CharacteristicCohomologyClass
if we pass it to the _element_constructor_
of DeRhamCohomologyRing
. But I suppose that is against the coercion principles either, right?
comment:51 Changed 12 months ago by
- Commit changed from e17fe37d0ea5ecdd1e9932145b908037ed0261cd to 8e3263c69f8099d8dcbf9faf07999304642c7222
Branch pushed to git repo; I updated commit sha1. New commits:
8e3263c | Trac #29581: deprecate alias and more
|
comment:52 in reply to: ↑ 50 ; follow-up: ↓ 65 Changed 12 months ago by
Replying to gh-mjungmath:
Replying to tscrim:
Unfortunately this is over my head mathematically, so I don't think I can really help with possible solutions at this point.
Mh okay. If you like, we can talk about it privately at some point.
If you are still stuck with something, perhaps we can. Perhaps Éric can offer some wisdom here.
I have another question though. I tried to establish a coercion from the characteristic cohomology class ring to the de Rham cohomology ring. For that, I need a representative which I only get if I've inserted a connection. Now I've read that coercions must always suceed. So throwing an error if no representative is available is not an option then?
Probably. Although you can say there is a coercion, but the data used to initialize it has not been specified. However, this map must also be unique too (in a mathematical sense; I think different representatives for the same element is fine), which means you cannot change the connection. This might be a place where we can justify having an exception because of the surrounding framework, but it is likely better to avoid that and make the user explicitly ask for a conversion.
Otherwise, we could return the same instance of
CharacteristicCohomologyClass
if we pass it to the_element_constructor_
ofDeRhamCohomologyRing
. But I suppose that is against the coercion principles either, right?
If we are passing it to the _element_cosntructor_
, then it is a conversion, and there are essentially no rules at that point.
comment:53 Changed 12 months ago by
Thank you Travis for your comments. My original idea was to inherit from DeRhamCohomologyClass
. Because both classes have the representative
method in common, which is crucial to perform operations such as additions and cup-products. My "dream" would be that an element of CharacteristicCohomologyClassRing
can be seen as member of DeRhamCohomologyRing
, and whenever representative
is invoked (even after coercions), you can still choose a connection.
One way to achieve this could be to allow the attribute _representative
in DeRhamCohomologyClass
to refer to a method. But I suppose this is rather spurious for the user?
Alternatively, if we drop my "dream" and still allow coercion: we can modify DeRhamCohomologyClass
in such a way that we can give it a name and allow to leave the _representative
attribute empty. Similar to scalar fields whose expressions are not set yet. Afterwards, the representative can be set dynamically, and we can make the instance immutable if desired.
What would you prefer Travis? Eric, do you have an opinion, too?
comment:54 Changed 11 months ago by
- Commit changed from 8e3263c69f8099d8dcbf9faf07999304642c7222 to 74d324f37e20c80488d38f1f4e8c22c0a3e02108
Branch pushed to git repo; I updated commit sha1. New commits:
74d324f | Trac #29851: add sequences
|
comment:55 Changed 11 months ago by
- Commit changed from 74d324f37e20c80488d38f1f4e8c22c0a3e02108 to 1f51ca73c5e5ab0ac1a5e5361b1100198c765dd1
Branch pushed to git repo; I updated commit sha1. New commits:
1f51ca7 | Trac #29851: sequences as static functions
|
comment:56 Changed 11 months ago by
- Commit changed from 1f51ca73c5e5ab0ac1a5e5361b1100198c765dd1 to ffa79bb0e94248699da3b73946310aa8eefa9516
Branch pushed to git repo; I updated commit sha1. New commits:
ffa79bb | Trac #29851: improve doc
|
comment:57 Changed 11 months ago by
- Commit changed from ffa79bb0e94248699da3b73946310aa8eefa9516 to 54cf11fa0f701f76523b1da5bf0e1434d9ab1723
Branch pushed to git repo; I updated commit sha1. New commits:
54cf11f | Trac #29851: factory class
|
comment:58 Changed 11 months ago by
- Commit changed from 54cf11fa0f701f76523b1da5bf0e1434d9ab1723 to d0f14dc7caeb6fb7a225d73939af6f5a95e06ca7
comment:59 Changed 11 months ago by
I think I made the constructor class CharacteristicCohomologyClass
unnecessarily complicated. Do you have an idea to make it more clean?
Otherwise, the main parts should be finished except for coercion support w.r.t. de Rham cohomology (see comment:53).
comment:60 Changed 11 months ago by
- Commit changed from d0f14dc7caeb6fb7a225d73939af6f5a95e06ca7 to 4b371bd075fc8c82328cffeff08c7cee91bfa9b5
Branch pushed to git repo; I updated commit sha1. New commits:
4b371bd | Trac #29581: add AHat class + minor fixes
|
comment:61 Changed 11 months ago by
- Commit changed from 4b371bd075fc8c82328cffeff08c7cee91bfa9b5 to e929409c7709091b1d3627614b7042f7b432274e
Branch pushed to git repo; I updated commit sha1. New commits:
e929409 | Trac #29581: remove cached_function due to dynamic behavior w.r.t. has_orientation
|
comment:62 Changed 11 months ago by
- Commit changed from e929409c7709091b1d3627614b7042f7b432274e to 4b371bd075fc8c82328cffeff08c7cee91bfa9b5
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
comment:63 Changed 11 months ago by
- Commit changed from 4b371bd075fc8c82328cffeff08c7cee91bfa9b5 to e82168f67b13aec1f32208eef2a2bf17b7e6fc04
Branch pushed to git repo; I updated commit sha1. New commits:
e82168f | Trac #29581: minor improvements
|
comment:64 Changed 11 months ago by
- Commit changed from e82168f67b13aec1f32208eef2a2bf17b7e6fc04 to 9f53f25b684540bf08e1d64d8a31b9420ebcce25
Branch pushed to git repo; I updated commit sha1. New commits:
9f53f25 | Trac #29581: adapt names
|
comment:65 in reply to: ↑ 52 Changed 11 months ago by
Replying to tscrim:
Probably. Although you can say there is a coercion, but the data used to initialize it has not been specified. However, this map must also be unique too (in a mathematical sense; I think different representatives for the same element is fine), which means you cannot change the connection.
Wait. The cohomology class is independent of the connection. It just gives different representatives. Then it should be fine.
comment:66 Changed 11 months ago by
- Commit changed from 9f53f25b684540bf08e1d64d8a31b9420ebcce25 to 86b4d28a2aceb68e36b6fed8ca4ee1d8dcbc754c
Branch pushed to git repo; I updated commit sha1. New commits:
86b4d28 | Trac #29581: add coercion and further improvements
|
comment:67 Changed 11 months ago by
- Commit changed from 86b4d28a2aceb68e36b6fed8ca4ee1d8dcbc754c to f81b25d80d4b0b9c692b25af037f1fe7623e96e8
Branch pushed to git repo; I updated commit sha1. New commits:
f81b25d | Trac #39581: add example for coercion
|
comment:68 Changed 11 months ago by
- Commit changed from f81b25d80d4b0b9c692b25af037f1fe7623e96e8 to 69dd5301343ea310bbdec34b65585a837c18be1b
Branch pushed to git repo; I updated commit sha1. New commits:
69dd530 | Trac #39581: improve doc
|
comment:69 Changed 11 months ago by
- Commit changed from 69dd5301343ea310bbdec34b65585a837c18be1b to 00203e394d5afd7b61802a39dcf6cde13f73efb7
Branch pushed to git repo; I updated commit sha1. New commits:
7328d50 | Trac #32272: remove unused imports
|
970d295 | Trac #30473: unicode -> latex for cup product
|
5021707 | Merge branch 't/32272/finitely_generated_graded_algebras_with_finite_degree' into t/29581/new_algorithm
|
00203e3 | Trac #29581: use cup-product for mul_symbol
|
comment:70 Changed 11 months ago by
- Commit changed from 00203e394d5afd7b61802a39dcf6cde13f73efb7 to 7555772e7b26039de4796997cd6f812a287ba49e
Branch pushed to git repo; I updated commit sha1. New commits:
7555772 | Trac #29581: copy doc from old file + add some doc for CharacteristicCohomologyRing
|
comment:71 Changed 11 months ago by
- Commit changed from 7555772e7b26039de4796997cd6f812a287ba49e to 5f542e15b88799d5e37df351d32a6389e237e913
Branch pushed to git repo; I updated commit sha1. New commits:
5f542e1 | Trac #29581: improve doc CharacteristicCohomologyClassRing
|
comment:72 Changed 11 months ago by
- Commit changed from 5f542e15b88799d5e37df351d32a6389e237e913 to 0ca6eb9fac729f9cfe68f61a5a41bf8e3d1f6504
Branch pushed to git repo; I updated commit sha1. New commits:
0ca6eb9 | Trac #29581: add doc to sequences
|
comment:73 Changed 11 months ago by
- Commit changed from 0ca6eb9fac729f9cfe68f61a5a41bf8e3d1f6504 to e7169f81d27242663c168350b203c2d8d51861ef
Branch pushed to git repo; I updated commit sha1. New commits:
e7169f8 | Trac #29581: doc of fast_wedge_power
|
comment:74 Changed 11 months ago by
- Commit changed from e7169f81d27242663c168350b203c2d8d51861ef to 114b483325526810c504935048e87b1e813985df
Branch pushed to git repo; I updated commit sha1. New commits:
114b483 | Trac #29581: add more docstring
|
comment:75 Changed 11 months ago by
- Commit changed from 114b483325526810c504935048e87b1e813985df to bafd78dcbb8e5b385051d64cc0a68a2e14908ad6
Branch pushed to git repo; I updated commit sha1. New commits:
bafd78d | Trac #29581: remove old file + add more doc
|
comment:76 Changed 11 months ago by
- Commit changed from bafd78dcbb8e5b385051d64cc0a68a2e14908ad6 to 265f918b8045a6b93142130f9fa4147fc7bd1f4c
Branch pushed to git repo; I updated commit sha1. New commits:
265f918 | Trac #29581: fix sign mistake
|
comment:77 Changed 11 months ago by
- Commit changed from 265f918b8045a6b93142130f9fa4147fc7bd1f4c to a3555e13332e0879adae188179120f4311462c6e
Branch pushed to git repo; I updated commit sha1. New commits:
a3555e1 | Trac #29581: more docstring
|
comment:78 Changed 11 months ago by
- Commit changed from a3555e13332e0879adae188179120f4311462c6e to 939cfd9d21342527dc0442f653dc50791ce73ad8
Branch pushed to git repo; I updated commit sha1. New commits:
939cfd9 | Trac #29581: more doc EulerAlgorithm
|
comment:79 Changed 11 months ago by
- Commit changed from 939cfd9d21342527dc0442f653dc50791ce73ad8 to e8d8393314a45980c25d110734a57ac4bda64191
Branch pushed to git repo; I updated commit sha1. New commits:
e8d8393 | Trac #29581: correct doc for EulerAlgorithm
|
comment:80 Changed 11 months ago by
- Commit changed from e8d8393314a45980c25d110734a57ac4bda64191 to 974c3cc866d51d884de7b666335a8c5d36517666
Branch pushed to git repo; I updated commit sha1. New commits:
974c3cc | Trac #29851: finish doc so far
|
comment:81 Changed 11 months ago by
- Cc gh-tobiasdiez added
- Status changed from needs_info to needs_review
Ticket should be finally ready for review now. :)
comment:82 Changed 11 months ago by
By the way, comparing wall times within the notebook https://nbviewer.jupyter.org/github/sagemanifolds/SageManifolds/blob/master/Notebooks/SM_char_classes.ipynb yields very good results!
comment:83 Changed 11 months ago by
- Commit changed from 974c3cc866d51d884de7b666335a8c5d36517666 to 5830550c072e7ba16215dfe80e3985cee9e7d320
Branch pushed to git repo; I updated commit sha1. New commits:
5830550 | Trac #29581: fix :: mistake and add Singleton example
|
comment:84 Changed 11 months ago by
Patchbot is morally green.
comment:85 Changed 11 months ago by
- Commit changed from 5830550c072e7ba16215dfe80e3985cee9e7d320 to 9a73cad1f5c1504b62c1890b28cc12c753cf9b23
Branch pushed to git repo; I updated commit sha1. New commits:
9a73cad | Trac #29581: correct ideal in doc + add long time tags for 30s or more
|
comment:86 Changed 11 months ago by
- Commit changed from 9a73cad1f5c1504b62c1890b28cc12c753cf9b23 to 4c1bc44977bbf506a415c9635bf884ae0519e229
Branch pushed to git repo; I updated commit sha1. New commits:
4c1bc44 | Trac #29581: remove f-string without placeholder
|
comment:87 Changed 11 months ago by
- Commit changed from 4c1bc44977bbf506a415c9635bf884ae0519e229 to e8e0a0b3040b3a2e750eec48cf7ffbd4d3cfb65f
Branch pushed to git repo; I updated commit sha1. New commits:
e8e0a0b | Merge branch 'develop' into t/29581/new_algorithm
|
comment:88 follow-up: ↓ 90 Changed 11 months ago by
- Reviewers set to Travis Scrimshaw
Since CharacteristicCohomologyClass
is not a function nor user-facing, you should follow Python naming conventions and write it as characteristic_cohomology_class
, perhaps adding build_
in front. However, I think you are better off tying that to the ring, perhaps as a (cached) helper method. This ties the cache of classes to the lifecycle of the ring rather than globally (which also nails the rings in memory) and better links everything from a code-association perspective.
This isn't necessarily specific to this ticket (but maybe now with these changes this is now possible), but do we have an example where the entire cohomology ring of a manifold is computed, such as the sphere or torus?
Perhaps as a followup ticket, a thematic tutorial on how to compute with the cohomology classes of the manifold might be good (using the module-level doc as a starting point).
comment:89 Changed 11 months ago by
- Commit changed from e8e0a0b3040b3a2e750eec48cf7ffbd4d3cfb65f to ec282634db5fbb61c7731ba8998c356814887f70
Branch pushed to git repo; I updated commit sha1. New commits:
ec28263 | Trac #29581: tie construuctor to ring
|
comment:90 in reply to: ↑ 88 ; follow-up: ↓ 92 Changed 11 months ago by
Replying to tscrim:
Since
CharacteristicCohomologyClass
is not a function nor user-facing, you should follow Python naming conventions and write it ascharacteristic_cohomology_class
, perhaps addingbuild_
in front. However, I think you are better off tying that to the ring, perhaps as a (cached) helper method. This ties the cache of classes to the lifecycle of the ring rather than globally (which also nails the rings in memory) and better links everything from a code-association perspective.
Thank you Travis for this feedback. I also tied it to the element constructor now. Is that what you had in mind?
This isn't necessarily specific to this ticket (but maybe now with these changes this is now possible), but do we have an example where the entire cohomology ring of a manifold is computed, such as the sphere or torus?
Take for example CP^1 = S^2
whose 2nd cohomology is generated by the first Chern class of the tautological bundle. But this is a rather special example.
Perhaps as a followup ticket, a thematic tutorial on how to compute with the cohomology classes of the manifold might be good (using the module-level doc as a starting point).
module-level doc?
comment:91 Changed 11 months ago by
- Commit changed from ec282634db5fbb61c7731ba8998c356814887f70 to c5370ec74eb85fd267c558ea8bad0e74512402b1
Branch pushed to git repo; I updated commit sha1. New commits:
c5370ec | Trac #29581: remove line breaks due to hard wrap in editor
|
comment:92 in reply to: ↑ 90 ; follow-up: ↓ 93 Changed 11 months ago by
Replying to gh-mjungmath:
Replying to tscrim:
Since
CharacteristicCohomologyClass
is not a function nor user-facing, you should follow Python naming conventions and write it ascharacteristic_cohomology_class
, perhaps addingbuild_
in front. However, I think you are better off tying that to the ring, perhaps as a (cached) helper method. This ties the cache of classes to the lifecycle of the ring rather than globally (which also nails the rings in memory) and better links everything from a code-association perspective.Thank you Travis for this feedback. I also tied it to the element constructor now. Is that what you had in mind?
Yea, that looks good. Thank you.
This isn't necessarily specific to this ticket (but maybe now with these changes this is now possible), but do we have an example where the entire cohomology ring of a manifold is computed, such as the sphere or torus?
Take for example
CP^1 = S^2
whose 2nd cohomology is generated by the first Chern class of the tautological bundle. But this is a rather special example.
Perhaps that is a bit special. The torus is a little more interesting example. Mainly what I am thinking is if can we compute the Betti numbers without necessarily knowing what the manifold is in advance.
Perhaps as a followup ticket, a thematic tutorial on how to compute with the cohomology classes of the manifold might be good (using the module-level doc as a starting point).
module-level doc?
The documentation at the top of the characteristic_cohomology_class.py
file (a Python module).
comment:93 in reply to: ↑ 92 ; follow-up: ↓ 94 Changed 11 months ago by
Replying to tscrim:
Yea, that looks good. Thank you.
So, what's missing for a positive review then?
Perhaps that is a bit special. The torus is a little more interesting example. Mainly what I am thinking is if can we compute the Betti numbers without necessarily knowing what the manifold is in advance.
I don't see how that should be possible with this implementation. Characteristic cohomology classes usually don't tell you anything about the cohomology of your underlying base space. For once, the characteristic cohomology classes in general don't generate the cohomology groups. Moreover, if a characteristic cohomology class has torsion in the ZZ-cohomology, its corresponding class in the de Rham cohomology vanishes. So you lose a lot of information on the way.
Perhaps as a followup ticket, a thematic tutorial on how to compute with the cohomology classes of the manifold might be good (using the module-level doc as a starting point).
module-level doc?
The documentation at the top of the
characteristic_cohomology_class.py
file (a Python module).
Right. But this is indeed something for a follow-up. However, don't you think this fits better into a general tutorial?
comment:94 in reply to: ↑ 93 Changed 11 months ago by
- Status changed from needs_review to positive_review
Replying to gh-mjungmath:
Replying to tscrim:
Yea, that looks good. Thank you.
So, what's missing for a positive review then?
You've answered all of my other questions, so nothing now. Thank you.
Perhaps that is a bit special. The torus is a little more interesting example. Mainly what I am thinking is if can we compute the Betti numbers without necessarily knowing what the manifold is in advance.
I don't see how that should be possible with this implementation. Characteristic cohomology classes usually don't tell you anything about the cohomology of your underlying base space. For once, the characteristic cohomology classes in general don't generate the cohomology groups. Moreover, if a characteristic cohomology class has torsion in the ZZ-cohomology, its corresponding class in the de Rham cohomology vanishes. So you lose a lot of information on the way.
Okay. Thank you for the explanation.
Perhaps as a followup ticket, a thematic tutorial on how to compute with the cohomology classes of the manifold might be good (using the module-level doc as a starting point).
Right. But this is indeed something for a follow-up. However, don't you think this fits better into a general tutorial?
I agree that it should be a followup. Yet, it is a bit specialized, so I think a thematic tutorial is better.
comment:95 Changed 11 months ago by
Thank you for the review Travis! :)
comment:96 Changed 11 months ago by
- Status changed from positive_review to needs_work
Merge conflict
comment:97 Changed 11 months ago by
- Commit changed from c5370ec74eb85fd267c558ea8bad0e74512402b1 to 7777803bf6c305a8ed117972c8f08495aef90b7e
Branch pushed to git repo; I updated commit sha1. New commits:
7777803 | Merge branch 'develop' into t/29581/new_algorithm
|
comment:98 Changed 11 months ago by
I have no idea what caused the merge conflict. I hope it works now...
comment:99 Changed 11 months ago by
- Status changed from needs_work to needs_review
comment:100 Changed 10 months ago by
- Status changed from needs_review to positive_review
comment:101 Changed 10 months ago by
- Status changed from positive_review to needs_work
Sorry to interfere here, but the patchbot says
sage -t --long --random-seed=0 src/sage/manifolds/differentiable/characteristic_cohomology_class.py # 9 doctests failed
comment:102 Changed 10 months ago by
Looks like an import location changed:
ImportError: cannot import name 'I' from 'sage.libs.pynac.pynac'
comment:103 follow-up: ↓ 104 Changed 10 months ago by
This is probably due to #32386. How do I import the imaginary unit now?
comment:104 in reply to: ↑ 103 ; follow-up: ↓ 108 Changed 10 months ago by
Replying to gh-mjungmath:
This is probably due to #32386. How do I import the imaginary unit now?
sage: import_statements("I")
says
from sage.rings.imaginary_unit import I
Indeed:
sage: from sage.rings.imaginary_unit import I as I2 sage: I2 is I True
comment:105 Changed 10 months ago by
- Commit changed from 7777803bf6c305a8ed117972c8f08495aef90b7e to 4e2e8b85ee02476a51574d1e04e87b0e6c0eaa00
Branch pushed to git repo; I updated commit sha1. New commits:
4e2e8b8 | Trac #29581: new location of imaginary unit
|
comment:106 Changed 10 months ago by
- Status changed from needs_work to needs_review
Here we go. Let's hope this works now.
New commits:
4e2e8b8 | Trac #29581: new location of imaginary unit
|
comment:107 follow-up: ↓ 109 Changed 10 months ago by
From #32386, they made this change:
-from sage.libs.pynac.pynac import I +from sage.symbolic.expression import I
comment:108 in reply to: ↑ 104 Changed 10 months ago by
Replying to egourgoulhon:
Replying to gh-mjungmath:
This is probably due to #32386. How do I import the imaginary unit now?
sage: import_statements("I")
saysfrom sage.rings.imaginary_unit import IIndeed:
sage: from sage.rings.imaginary_unit import I as I2 sage: I2 is I True
BTW, I've just noticed that
sage: from sage.symbolic.constants import I as I3 sage: I3 is I False
However,
sage: bool(I3 == I) True sage: I3^2 -1 sage: I + I3 2*I
comment:109 in reply to: ↑ 107 Changed 10 months ago by
comment:110 Changed 10 months ago by
To summarize, there are at least 3 sources for I
: sage.rings.imaginary_unit
, sage.symbolic.constants
, sage.symbolic.expression
. All yield an sage.symbolic.expression.Expression
, but only the first one gives the same I
as the Python variable predefined in the interactive shell.
comment:111 Changed 10 months ago by
But sage.symbolic.constants
and sage.symbolic.expression
give the same I
, don't they?
comment:112 Changed 10 months ago by
#18036 is probably relevant here.
comment:113 follow-up: ↓ 114 Changed 10 months ago by
I think you want to follow the change from #32386 since you are wanting to do symbolic ring computations. Otherwise you end up with
sage: I.parent() Number Field in I with defining polynomial x^2 + 1 with I = 1*I
comment:114 in reply to: ↑ 113 Changed 10 months ago by
Replying to tscrim:
I think you want to follow the change from #32386 since you are wanting to do symbolic ring computations. Otherwise you end up with
sage: I.parent() Number Field in I with defining polynomial x^2 + 1 with I = 1*I
In src/sage/symbolic/all.py
there is a deprecation warning against from sage.symbolic.expression import I
:
lazy_import("sage.symbolic.expression", "I", deprecation=(18036, "import I from sage.symbolic.constants for the imaginary unit viewed as an element of SR, or from sage.rings.imaginary_unit for the element of ZZ[i]"))
However, this warning does not seem effective...
comment:115 Changed 10 months ago by
In a fresh Sage session, we have (since #18036 I guess)
sage: I.parent() Number Field in I with defining polynomial x^2 + 1 with I = 1*I sage: from sage.symbolic.constants import I as I_symb sage: I_symb.parent() Symbolic Ring sage: SR(I) is I_symb False
I would have expected True
for the last output (i.e. to have I
unique as a symbolic expression).
comment:116 Changed 10 months ago by
It's not so unnatural to have copies being made. It would also be a lot of work for SR to check that some potentially very complicated expression is equal to I
.
Okay, then follow the deprecation and import from sage.symbolic.constants
. The main thing is to not use the one from from sage.rings.imaginary_unit import I
.
comment:117 Changed 10 months ago by
- Commit changed from 4e2e8b85ee02476a51574d1e04e87b0e6c0eaa00 to 44f8cfa8d960c1ec2db3a097f95728efcc86fcf4
Branch pushed to git repo; I updated commit sha1. New commits:
44f8cfa | Trac #29581: import I from symbolic.constants
|
comment:118 Changed 10 months ago by
Okay, there we go. Thank you for this feedback!
comment:119 Changed 10 months ago by
- Reviewers changed from Travis Scrimshaw to Travis Scrimshaw, Eric Gourgoulhon
comment:120 Changed 10 months ago by
- Status changed from needs_review to positive_review
Green bot. LGTM.
comment:121 Changed 10 months ago by
- Status changed from positive_review to needs_work
[docpdf] ! Package inputenc Error: Unicode character ϑ (U+03D1) [docpdf] (inputenc) not set up for use with LaTeX. [docpdf] [docpdf] See the inputenc package documentation for explanation. [docpdf] Type H <return> for immediate help. [docpdf] ... [docpdf] [docpdf] l.73283 ...1}{\PYGZsh{} use spherical coordinates} [docpdf] [docpdf] ? [docpdf] ! Emergency stop.
comment:122 Changed 10 months ago by
Indeed, any new Unicode character has to be declared in
src/sage/docs/conf.py
for the pdf doc to be generated successfully.
comment:123 Changed 10 months ago by
- Commit changed from 44f8cfa8d960c1ec2db3a097f95728efcc86fcf4 to ac5cbbf0072380022c32466aab95fdfb31c4ecee
Branch pushed to git repo; I updated commit sha1. New commits:
ac5cbbf | Trac #29581: remove unicode character
|
comment:124 Changed 10 months ago by
- Status changed from needs_work to needs_review
That should solve it. Hopefully.
comment:125 Changed 10 months ago by
- Status changed from needs_review to positive_review
That is one way to solve it.
comment:126 Changed 10 months ago by
- Branch changed from u/gh-mjungmath/new_algorithm to ac5cbbf0072380022c32466aab95fdfb31c4ecee
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Trac #29570: alternating_form returns correct element
Trac #29570: Typo fixed, doctest added, returned element preferably non-zero
Trac #29581: New Algorithm for Char Classes