Opened 2 years ago

Closed 12 months ago

#29581 closed enhancement (fixed)

New Algorithm for Characteristic Classes

Reported by: Michael Jung Owned by:
Priority: major Milestone: sage-9.5
Component: manifolds Keywords: characteristic_classes, manifolds
Cc: Samuel Lelièvre, Eric Gourgoulhon, Travis Scrimshaw, Matthias Köppe, Tobias Diez 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:

Status badges

Description (last modified by Michael Jung)

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 Michael Jung

Branch: u/gh-mjungmath/new_algorithm

comment:2 Changed 2 years ago by Matthias Köppe

Commit: 1a803eba5b9724ebfa93e726a474d957fd6915b4
Milestone: sage-9.1sage-9.2

New commits:

3b36f0dTrac #29570: alternating_form returns correct element
85e7970Trac #29570: Typo fixed, doctest added, returned element preferably non-zero
1a803ebTrac #29581: New Algorithm for Char Classes

comment:3 Changed 2 years ago by Samuel Lelièvre

Cc: Samuel Lelièvre added
Component: PLEASE CHANGEgeometry

comment:4 Changed 2 years ago by Samuel Lelièvre

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 git

Commit: 1a803eba5b9724ebfa93e726a474d957fd6915b4cb442eb8919ca33a5db63a17bac52f99f55553dd

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

730046cMerge branch 'develop' into t/29570/diff_form_bug
130ae3fTrac #29570: NotImplementedError for non-generic ring elements
bba21e1Trac #29570: Strange typo fixed...
ba3b4b9Trac #29570: correct parent, vectorfield_module changes reverted
841e1bfMerge branch 't/29570/diff_form_bug' into t/29581/new_algorithm
cb442ebTrac #29570: new algorithm added and code cleaned

comment:6 Changed 2 years ago by git

Commit: cb442eb8919ca33a5db63a17bac52f99f55553dd15c7341d071327a04fd229c3d2f78d583564f476

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

15c7341Trac #29581: new algorithm added and code cleaned

comment:7 Changed 2 years ago by Michael Jung

Authors: Michael Jung
Cc: Eric Gourgoulhon added
Description: modified (diff)
Keywords: characteristic_classes manifolds added
Type: PLEASE CHANGEenhancement

comment:8 Changed 2 years ago by Frédéric Chapoton

Correct syntax for crosslinks is

.. SEEALSO::

comment:9 Changed 2 years ago by Frédéric Chapoton

  • 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 Michael Jung

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 Michael Jung

Thanks for your advice. :)

comment:12 Changed 2 years ago by git

Commit: 15c7341d071327a04fd229c3d2f78d583564f476d37229341effff46ea2fa4b3287e32997c8422b0

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

d372293Trac #29581: Strange typos reverted

comment:13 Changed 2 years ago by Michael Jung

Status: newneeds_review

comment:14 Changed 2 years ago by Michael Jung

Status: needs_reviewneeds_info

comment:15 Changed 2 years ago by Michael Jung

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 git

Commit: d37229341effff46ea2fa4b3287e32997c8422b019815c4257cee7dcf503d94e3b9bb651150bef5f

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

c975bd2Merge branch 'develop' into t/29581/new_algorithm
19815c4Trac #29581: Readability of one line

comment:17 Changed 2 years ago by Michael Jung

Component: geometrymanifolds
Dependencies: #30211

comment:18 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.2sage-9.3

comment:19 Changed 19 months ago by Matthias Köppe

Milestone: sage-9.3sage-9.4

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.

comment:20 Changed 15 months ago by Matthias Köppe

Milestone: sage-9.4sage-9.5

Setting a new milestone for this ticket based on a cursory review.

comment:21 Changed 15 months ago by Michael Jung

Cc: Travis Scrimshaw Matthias Köppe added
Dependencies: #30211
Description: modified (diff)
Status: needs_infoneeds_work

comment:22 Changed 15 months ago by git

Commit: 19815c4257cee7dcf503d94e3b9bb651150bef5f3c9b80a2551994493f23640344e9ade63240cb01

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

3c9b80aTrac #29581: implement algorithms for chern, pontryagin and euler

comment:23 Changed 15 months ago by Michael Jung

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 15 months ago by Michael Jung

Description: modified (diff)

comment:25 Changed 15 months ago by Michael Jung

Status: needs_workneeds_info

comment:26 Changed 15 months ago by Michael Jung

Description: modified (diff)

comment:27 Changed 15 months ago by git

Commit: 3c9b80a2551994493f23640344e9ade63240cb0103529f8919357f98b65ee033e8dc5c7d48b2fa42

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

03529f8Trac #29581: return mixed form

comment:28 Changed 14 months ago by git

Commit: 03529f8919357f98b65ee033e8dc5c7d48b2fa42ab288d62786a40b9f314d40ca932506d7b6932d9

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

fb9c661#32270: de rham cohomology as algebra
de5863aMerge branch 't/32270/turn_de_rham_cohomology_into_algebra' into t/29581/new_algorithm
d22ee9aTrac #32315: support iteration and enumerated sets
16c4737Trac #32272: allow infinite max degree
6a320a0Trac #32272: fix docstring
c1d4fceTrac #32272: fix rst file + rename module
25e6b1dTrac #32272: fix doctest
5a4c7acTrac #32272: narrow to finite dimensions
3b81085Merge branch 't/32272/finitely_generated_graded_algebras_with_finite_degree' into t/29581/new_algorithm
ab288d6Trac #29581: algorithm for complex vbundles

comment:29 Changed 14 months ago by Michael Jung

Dependencies: #32270, #32272

comment:30 Changed 14 months ago by git

Commit: ab288d62786a40b9f314d40ca932506d7b6932d96688740017097a79d7995101889cea097ee5c653

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

932e128Trac #29581: element constructor
6688740Trac #29581: naming

comment:31 Changed 14 months ago by git

Commit: 6688740017097a79d7995101889cea097ee5c65361291f406b886abb9a509955355da749fc99a8d6

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

61291f4Trac #29581: minor fixes

comment:32 Changed 14 months ago by git

Commit: 61291f406b886abb9a509955355da749fc99a8d660318775e9c530c32280ba1bfa819cb9ac978bd1

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

6031877Trac #29581: return only list of generators

comment:33 Changed 14 months ago by git

Commit: 60318775e9c530c32280ba1bfa819cb9ac978bd10c7587f759b9555404fd417ee042e99fbb4ae02d

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

0c7587fTrac #29581: parent deals with different cases, element stays generic

comment:34 Changed 14 months ago by git

Commit: 0c7587f759b9555404fd417ee042e99fbb4ae02d28d2d84271a3e1516556a8364b1114dd6edaa992

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

28d2d84Trac #29581: revert mistaken merge

comment:35 Changed 14 months ago by Michael Jung

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

Commit: 28d2d84271a3e1516556a8364b1114dd6edaa9922cabd839d55e5c0f3ba2f8d0194bcae6ce31397c

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

2cabd83Trac #29581: add some comments

comment:37 Changed 14 months ago by Michael Jung

Dependencies: #32270, #32272#32270, #32272, #32396

comment:38 Changed 14 months ago by Michael Jung

Description: modified (diff)

comment:39 Changed 14 months ago by git

Commit: 2cabd839d55e5c0f3ba2f8d0194bcae6ce31397c815392c4bcf2a6177cecbcc24ae3dc73372fe743

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

1a9b7d9Trac #29581: compute Euler forms for arbitrary positively oriented frames
fcd64c9Trac #32396: add __abs__ to chart_func and scalarfield
6d70be4Merge branch 'abs_function_for_scalar_fields' into t/29581/new_algorithm
815392cTrac #29581: fix some bugs + treat Euler case

comment:40 Changed 14 months ago by git

Commit: 815392c4bcf2a6177cecbcc24ae3dc73372fe74362ff48af88ee24979df68297551a540a47dbc1a3

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

62ff48aTrac #29581: leave comment on to-do

comment:41 Changed 14 months ago by git

Commit: 62ff48af88ee24979df68297551a540a47dbc1a35b8632d7b5ac52bfe943660b26ffabe35b2c7a8f

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

5b8632dTrac #29581: euler form algorithm finished

comment:42 Changed 14 months ago by git

Commit: 5b8632d7b5ac52bfe943660b26ffabe35b2c7a8f4afc9f0d0ce1212516c67f6d04617151ef4d0ea3

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

4afc9f0Trac #29581: compute forms only on demand

comment:43 Changed 14 months ago by Michael Jung

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

Commit: 4afc9f0d0ce1212516c67f6d04617151ef4d0ea3201831f1338973a6c5eefb6c9ee61592f9e38f0f

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

201831fTrac #29581: commenting improved

comment:45 Changed 14 months ago by Michael Jung

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 14 months ago by Travis Scrimshaw

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 14 months ago by Michael Jung

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.

Last edited 14 months ago by Michael Jung (previous) (diff)

comment:48 Changed 14 months ago by git

Commit: 201831f1338973a6c5eefb6c9ee61592f9e38f0fe17fe37d0ea5ecdd1e9932145b908037ed0261cd

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

e17fe37Trac #29581: comments further improved + _repr_ method

comment:49 Changed 14 months ago by Travis Scrimshaw

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 ; Changed 14 months ago by Michael Jung

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

Commit: e17fe37d0ea5ecdd1e9932145b908037ed0261cd8e3263c69f8099d8dcbf9faf07999304642c7222

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

8e3263cTrac #29581: deprecate alias and more

comment:52 in reply to:  50 ; Changed 13 months ago by Travis Scrimshaw

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_ of DeRhamCohomologyRing. 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 13 months ago by Michael Jung

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?

Last edited 13 months ago by Michael Jung (previous) (diff)

comment:54 Changed 13 months ago by git

Commit: 8e3263c69f8099d8dcbf9faf07999304642c722274d324f37e20c80488d38f1f4e8c22c0a3e02108

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

74d324fTrac #29851: add sequences

comment:55 Changed 13 months ago by git

Commit: 74d324f37e20c80488d38f1f4e8c22c0a3e021081f51ca73c5e5ab0ac1a5e5361b1100198c765dd1

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

1f51ca7Trac #29851: sequences as static functions

comment:56 Changed 13 months ago by git

Commit: 1f51ca73c5e5ab0ac1a5e5361b1100198c765dd1ffa79bb0e94248699da3b73946310aa8eefa9516

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

ffa79bbTrac #29851: improve doc

comment:57 Changed 13 months ago by git

Commit: ffa79bb0e94248699da3b73946310aa8eefa951654cf11fa0f701f76523b1da5bf0e1434d9ab1723

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

54cf11fTrac #29851: factory class

comment:58 Changed 13 months ago by git

Commit: 54cf11fa0f701f76523b1da5bf0e1434d9ab1723d0f14dc7caeb6fb7a225d73939af6f5a95e06ca7

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

ba71d70Trac #29581: turn polynomial into dictionary
5c0f165Merge branch 'develop' into t/29581/new_algorithm
d0f14dcTrac #29581: constructor class finished

comment:59 Changed 13 months ago by Michael Jung

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

Commit: d0f14dc7caeb6fb7a225d73939af6f5a95e06ca74b371bd075fc8c82328cffeff08c7cee91bfa9b5

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

4b371bdTrac #29581: add AHat class + minor fixes

comment:61 Changed 13 months ago by git

Commit: 4b371bd075fc8c82328cffeff08c7cee91bfa9b5e929409c7709091b1d3627614b7042f7b432274e

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

e929409Trac #29581: remove cached_function due to dynamic behavior w.r.t. has_orientation

comment:62 Changed 13 months ago by git

Commit: e929409c7709091b1d3627614b7042f7b432274e4b371bd075fc8c82328cffeff08c7cee91bfa9b5

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

comment:63 Changed 13 months ago by git

Commit: 4b371bd075fc8c82328cffeff08c7cee91bfa9b5e82168f67b13aec1f32208eef2a2bf17b7e6fc04

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

e82168fTrac #29581: minor improvements

comment:64 Changed 13 months ago by git

Commit: e82168f67b13aec1f32208eef2a2bf17b7e6fc049f53f25b684540bf08e1d64d8a31b9420ebcce25

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

9f53f25Trac #29581: adapt names

comment:65 in reply to:  52 Changed 13 months ago by Michael Jung

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

Commit: 9f53f25b684540bf08e1d64d8a31b9420ebcce2586b4d28a2aceb68e36b6fed8ca4ee1d8dcbc754c

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

86b4d28Trac #29581: add coercion and further improvements

comment:67 Changed 13 months ago by git

Commit: 86b4d28a2aceb68e36b6fed8ca4ee1d8dcbc754cf81b25d80d4b0b9c692b25af037f1fe7623e96e8

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

f81b25dTrac #39581: add example for coercion

comment:68 Changed 13 months ago by git

Commit: f81b25d80d4b0b9c692b25af037f1fe7623e96e869dd5301343ea310bbdec34b65585a837c18be1b

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

69dd530Trac #39581: improve doc

comment:69 Changed 13 months ago by git

Commit: 69dd5301343ea310bbdec34b65585a837c18be1b00203e394d5afd7b61802a39dcf6cde13f73efb7

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

7328d50Trac #32272: remove unused imports
970d295Trac #30473: unicode -> latex for cup product
5021707Merge branch 't/32272/finitely_generated_graded_algebras_with_finite_degree' into t/29581/new_algorithm
00203e3Trac #29581: use cup-product for mul_symbol

comment:70 Changed 13 months ago by git

Commit: 00203e394d5afd7b61802a39dcf6cde13f73efb77555772e7b26039de4796997cd6f812a287ba49e

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

7555772Trac #29581: copy doc from old file + add some doc for CharacteristicCohomologyRing

comment:71 Changed 13 months ago by git

Commit: 7555772e7b26039de4796997cd6f812a287ba49e5f542e15b88799d5e37df351d32a6389e237e913

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

5f542e1Trac #29581: improve doc CharacteristicCohomologyClassRing

comment:72 Changed 13 months ago by git

Commit: 5f542e15b88799d5e37df351d32a6389e237e9130ca6eb9fac729f9cfe68f61a5a41bf8e3d1f6504

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

0ca6eb9Trac #29581: add doc to sequences

comment:73 Changed 13 months ago by git

Commit: 0ca6eb9fac729f9cfe68f61a5a41bf8e3d1f6504e7169f81d27242663c168350b203c2d8d51861ef

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

e7169f8Trac #29581: doc of fast_wedge_power

comment:74 Changed 13 months ago by git

Commit: e7169f81d27242663c168350b203c2d8d51861ef114b483325526810c504935048e87b1e813985df

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

114b483Trac #29581: add more docstring

comment:75 Changed 13 months ago by git

Commit: 114b483325526810c504935048e87b1e813985dfbafd78dcbb8e5b385051d64cc0a68a2e14908ad6

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

bafd78dTrac #29581: remove old file + add more doc

comment:76 Changed 13 months ago by git

Commit: bafd78dcbb8e5b385051d64cc0a68a2e14908ad6265f918b8045a6b93142130f9fa4147fc7bd1f4c

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

265f918Trac #29581: fix sign mistake

comment:77 Changed 13 months ago by git

Commit: 265f918b8045a6b93142130f9fa4147fc7bd1f4ca3555e13332e0879adae188179120f4311462c6e

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

a3555e1Trac #29581: more docstring

comment:78 Changed 13 months ago by git

Commit: a3555e13332e0879adae188179120f4311462c6e939cfd9d21342527dc0442f653dc50791ce73ad8

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

939cfd9Trac #29581: more doc EulerAlgorithm

comment:79 Changed 13 months ago by git

Commit: 939cfd9d21342527dc0442f653dc50791ce73ad8e8d8393314a45980c25d110734a57ac4bda64191

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

e8d8393Trac #29581: correct doc for EulerAlgorithm

comment:80 Changed 13 months ago by git

Commit: e8d8393314a45980c25d110734a57ac4bda64191974c3cc866d51d884de7b666335a8c5d36517666

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

974c3ccTrac #29851: finish doc so far

comment:81 Changed 13 months ago by Michael Jung

Cc: Tobias Diez added
Status: needs_infoneeds_review

Ticket should be finally ready for review now. :)

comment:82 Changed 13 months ago by Michael Jung

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

Commit: 974c3cc866d51d884de7b666335a8c5d365176665830550c072e7ba16215dfe80e3985cee9e7d320

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

5830550Trac #29581: fix :: mistake and add Singleton example

comment:84 Changed 13 months ago by Michael Jung

Patchbot is morally green.

comment:85 Changed 13 months ago by git

Commit: 5830550c072e7ba16215dfe80e3985cee9e7d3209a73cad1f5c1504b62c1890b28cc12c753cf9b23

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

9a73cadTrac #29581: correct ideal in doc + add long time tags for 30s or more

comment:86 Changed 13 months ago by git

Commit: 9a73cad1f5c1504b62c1890b28cc12c753cf9b234c1bc44977bbf506a415c9635bf884ae0519e229

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

4c1bc44Trac #29581: remove f-string without placeholder

comment:87 Changed 13 months ago by git

Commit: 4c1bc44977bbf506a415c9635bf884ae0519e229e8e0a0b3040b3a2e750eec48cf7ffbd4d3cfb65f

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

e8e0a0bMerge branch 'develop' into t/29581/new_algorithm

comment:88 Changed 13 months ago by Travis Scrimshaw

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

Commit: e8e0a0b3040b3a2e750eec48cf7ffbd4d3cfb65fec282634db5fbb61c7731ba8998c356814887f70

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

ec28263Trac #29581: tie construuctor to ring

comment:90 in reply to:  88 ; Changed 13 months ago by Michael Jung

Replying to tscrim:

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.

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

Commit: ec282634db5fbb61c7731ba8998c356814887f70c5370ec74eb85fd267c558ea8bad0e74512402b1

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

c5370ecTrac #29581: remove line breaks due to hard wrap in editor

comment:92 in reply to:  90 ; Changed 13 months ago by Travis Scrimshaw

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

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 ; Changed 13 months ago by Michael Jung

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?

Last edited 13 months ago by Michael Jung (previous) (diff)

comment:94 in reply to:  93 Changed 12 months ago by Travis Scrimshaw

Status: needs_reviewpositive_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 12 months ago by Michael Jung

Thank you for the review Travis! :)

comment:96 Changed 12 months ago by Volker Braun

Status: positive_reviewneeds_work

Merge conflict

comment:97 Changed 12 months ago by git

Commit: c5370ec74eb85fd267c558ea8bad0e74512402b17777803bf6c305a8ed117972c8f08495aef90b7e

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

7777803Merge branch 'develop' into t/29581/new_algorithm

comment:98 Changed 12 months ago by Michael Jung

I have no idea what caused the merge conflict. I hope it works now...

comment:99 Changed 12 months ago by Michael Jung

Status: needs_workneeds_review

comment:100 Changed 12 months ago by Travis Scrimshaw

Status: needs_reviewpositive_review

comment:101 Changed 12 months ago by Eric Gourgoulhon

Status: positive_reviewneeds_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 12 months ago by Travis Scrimshaw

Looks like an import location changed:

ImportError: cannot import name 'I' from 'sage.libs.pynac.pynac'

comment:103 Changed 12 months ago by Michael Jung

This is probably due to #32386. How do I import the imaginary unit now?

comment:104 in reply to:  103 ; Changed 12 months ago by Eric Gourgoulhon

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

Commit: 7777803bf6c305a8ed117972c8f08495aef90b7e4e2e8b85ee02476a51574d1e04e87b0e6c0eaa00

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

4e2e8b8Trac #29581: new location of imaginary unit

comment:106 Changed 12 months ago by Michael Jung

Status: needs_workneeds_review

Here we go. Let's hope this works now.


New commits:

4e2e8b8Trac #29581: new location of imaginary unit

comment:107 Changed 12 months ago by Travis Scrimshaw

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 12 months ago by Eric Gourgoulhon

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") says

from sage.rings.imaginary_unit import I

Indeed:

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 12 months ago by Eric Gourgoulhon

Replying to tscrim:

From #32386, they made this change:

-from sage.libs.pynac.pynac import I
+from sage.symbolic.expression import I

Here we have:

sage: from sage.symbolic.expression import I as I4
sage: I4 is I
False

comment:110 Changed 12 months ago by Eric Gourgoulhon

To summarize, there are at least 3 sources for I: sage.rings.imaginary_unit, sage.symbolic.constants, sage.symbolic.expression, but only the first one gives the same I as the Python variable predefined in the interactive shell. Moreover, we have

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 I1
sage: I1.parent()
Symbolic Ring
sage: from sage.symbolic.expression import I as I2
sage: I2.parent()
Symbolic Ring
sage: I2 is I1
True
Last edited 12 months ago by Eric Gourgoulhon (previous) (diff)

comment:111 Changed 12 months ago by Michael Jung

But sage.symbolic.constants and sage.symbolic.expression give the same I, don't they?

comment:112 Changed 12 months ago by Eric Gourgoulhon

#18036 is probably relevant here.

comment:113 Changed 12 months ago by Travis Scrimshaw

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 12 months ago by Eric Gourgoulhon

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 12 months ago by Eric Gourgoulhon

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

Last edited 12 months ago by Eric Gourgoulhon (previous) (diff)

comment:116 Changed 12 months ago by Travis Scrimshaw

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

Commit: 4e2e8b85ee02476a51574d1e04e87b0e6c0eaa0044f8cfa8d960c1ec2db3a097f95728efcc86fcf4

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

44f8cfaTrac #29581: import I from symbolic.constants

comment:118 Changed 12 months ago by Michael Jung

Okay, there we go. Thank you for this feedback!

comment:119 Changed 12 months ago by Michael Jung

Reviewers: Travis ScrimshawTravis Scrimshaw, Eric Gourgoulhon

comment:120 Changed 12 months ago by Travis Scrimshaw

Status: needs_reviewpositive_review

Green bot. LGTM.

comment:121 Changed 12 months ago by Volker Braun

Status: positive_reviewneeds_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 12 months ago by Eric Gourgoulhon

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

Commit: 44f8cfa8d960c1ec2db3a097f95728efcc86fcf4ac5cbbf0072380022c32466aab95fdfb31c4ecee

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

ac5cbbfTrac #29581: remove unicode character

comment:124 Changed 12 months ago by Michael Jung

Status: needs_workneeds_review

That should solve it. Hopefully.

comment:125 Changed 12 months ago by Travis Scrimshaw

Status: needs_reviewpositive_review

That is one way to solve it.

comment:126 Changed 12 months ago by Volker Braun

Branch: u/gh-mjungmath/new_algorithmac5cbbf0072380022c32466aab95fdfb31c4ecee
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.