sagemath / sage

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

sage.geometry.hyperbolic_space: __matmul__ for composition of isometries #32212

Open mkoeppe opened 3 years ago

mkoeppe commented 3 years ago

Follow-up from #30244

Depends on #30244

CC: @tscrim

Component: geometry

Author: Matthias Koeppe

Branch/Commit: u/mkoeppe/sage_geometry_hyperbolic_space__use_actions_instead_of_overriding_mul____rmul__ @ 32c7d40

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

mkoeppe commented 3 years ago

Branch: u/mkoeppe/sage_geometry_hyperbolic_space__use_actions_instead_of_overriding_mul____rmul__

mkoeppe commented 3 years ago

Dependencies: #30244

mkoeppe commented 3 years ago

New commits:

1bb878cHyperbolicIsometry: Split out _composition from `__mul__` so that also @ between isometries works
923ebb7HyperbolicIsometryPD: Override _composition, not __mul__
mkoeppe commented 3 years ago

Commit: 923ebb7

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

Changed commit from 923ebb7 to 32c7d40

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

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

66206f9FanMorphism: Define `__matmul__`, not __mul__
3006833FiniteSetEndoMap*: Define `__matmul__`, delegate to it from __mul__
de14c34TensorWithIndices: Make `__matmul__` an alias of __mul__
451eb95Map.__mul__: Add doctest output
348f680TensorWithIndices: Update doctests
e808163Merge #30244
305029aCoercionModel.verify_coercion_maps: Use @ instead of * for composition
1a6671fPrecomposedAction.__init__: Use @ instead of * for composition
2d8698fHyperbolicGeodesic*: More @ instead of *
32c7d40HyperbolicIsometry.__matmul__: Repair composition of isometries from different models
mkoeppe commented 3 years ago

Author: Matthias Koeppe

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -1,2 +1,2 @@
-The current code is an obstacle for the matmul ticket #30244
+Follow-up from #30244
mkoeppe commented 2 years ago
comment:6

Stalled in needs_review or needs_info; likely won't make it into Sage 9.5.

mkoeppe commented 2 years ago
comment:7

needs rebase

grhkm21 commented 8 months ago

@mkoeppe What should I do if I want to unblock this ticket? Review it here or {make a new branch on my fork with the commits, rebase, then PR}? (I assume "rebase" means just git merge and fixing conflicts right?)

Also I realised you are the author, but I can help with rebasing.

mkoeppe commented 8 months ago

Yes, please feel free to take over. Preparing a PR is the best way forward. Whether you use git merge or git rebase to refresh the branch is up to you

grhkm21 commented 8 months ago

Will do 👍🏻

I didn’t know git rebase is a thing, oops.