Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sage.tensor.modules.free_module_basis: Make Basis_abstract a subclass of AbstractFamily #30300

Closed
mkoeppe opened this issue Aug 6, 2020 · 29 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 6, 2020

... in particular provide an implementation of the keys() method.

We also add a generic method AbstractFamily.items() to complement the methods keys() and values(), mimicking the Mapping protocol. (Split out from #34340.)

CC: @mjungmath @egourgoulhon @tscrim

Component: linear algebra

Author: Matthias Koeppe

Branch/Commit: 66874a9

Reviewer: Eric Gourgoulhon

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

@mkoeppe mkoeppe added this to the sage-9.2 milestone Aug 6, 2020
@mkoeppe mkoeppe modified the milestones: sage-9.2, sage-9.3 Aug 29, 2020
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 22, 2021

comment:2

Any help with implementing this would be very welcome

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 13, 2021

comment:3

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

@mkoeppe mkoeppe modified the milestones: sage-9.3, sage-9.4 Feb 13, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Jul 19, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.5, sage-9.6 Dec 14, 2021
@mkoeppe mkoeppe modified the milestones: sage-9.6, sage-9.7 Mar 5, 2022
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 18, 2022

Changed dependencies from #30287, #30259 to #30287, #30259, #34340

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 23, 2022

Changed dependencies from #30287, #30259, #34340 to #30287, #34340

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 23, 2022

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 23, 2022

New commits:

78400bcAbstractFamily.{keys,values,items}: New
d2e4466src/sage/sets/family.py: Add missing import
d15bd7dsrc/sage/tensor/modules/free_module_basis.py: Make Basis_abstract a subclass of AbstractFamily

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 23, 2022

Commit: d15bd7d

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 23, 2022

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 23, 2022

Changed dependencies from #30287, #34340 to none

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 23, 2022

Changed commit from d15bd7d to eeba29a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 23, 2022

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

eeba29asrc/sage/tensor/modules/free_module_basis.py: Add documentation

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 24, 2022

comment:14
sage -t --random-seed=263111727722887782759446361100915924402 sage/manifolds/differentiable/chart.py  # 2 doctests failed
sage -t --random-seed=263111727722887782759446361100915924402 sage/manifolds/differentiable/vectorframe.py  # 2 doctests failed
sage -t --random-seed=263111727722887782759446361100915924402 sage/manifolds/local_frame.py  # 2 doctests failed

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 24, 2022

Changed commit from eeba29a to 5936c6b

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 24, 2022

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

5936c6bsrc/sage/manifolds: Update doctest outputs

@egourgoulhon
Copy link
Member

comment:16

Could you please remind me what is the purpose of this ticket? In particular, why should bases of a FiniteRankFreeModule have a keys() method?

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 25, 2022

Changed commit from 5936c6b to 66874a9

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 25, 2022

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

66874a9src/sage/sets/family.py: Add doctest

@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@egourgoulhon
Copy link
Member

comment:21

Replying to @mkoeppe:

Replying to @egourgoulhon:

Could you please remind me what is the purpose of this ticket? In particular, why should bases of a FiniteRankFreeModule have a keys() method?

By itself, this change only improves "discoverability": If a user has an instance of Basis_abstract, currently they can get an element by calling __getitem__ and either get an element or an error. (Or they would have to know the detail that the indices accepted by __getitem__ are the underlying modules's irange.)

It is also preparation for #30229, which makes the standard bases of tensor modules (induced by the bases of their base module) explicit objects (instances of the subclass TensorFreeSubmoduleBasis_comp), instead of just being implicit in various algorithms.

Thanks for your answer. It's still not clear to me what the advantage of making Basis_abstract a subclass of AbstractFamily is. It adds some code complexity, making FreeModuleBasis be a Parent, but I trust you it is worth it. Could you tell which functionality of AbstractFamily or FiniteEnumeratedSets (the declared category) do you plan to use in #30229?

@egourgoulhon egourgoulhon changed the title sage.tensor.modules.free_module_basis: Make Basis_abstract a subclass of Family sage.tensor.modules.free_module_basis: Make Basis_abstract a subclass of AbstractFamily Aug 26, 2022
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 27, 2022

comment:23

Replying to @egourgoulhon:

Could you tell which functionality of AbstractFamily or FiniteEnumeratedSets (the declared category) do you plan to use in #30229?

The new version of method isomorphism_with_fixed_basis in #34427 illustrates what I have in mind

@egourgoulhon
Copy link
Member

Reviewer: Eric Gourgoulhon

@egourgoulhon
Copy link
Member

comment:24

Replying to @mkoeppe:

Replying to @egourgoulhon:

Could you tell which functionality of AbstractFamily or FiniteEnumeratedSets (the declared category) do you plan to use in #30229?

The new version of method isomorphism_with_fixed_basis in #34427 illustrates what I have in mind

It looks that this uses only the method keys(), which can be defined without any reference to AbstractFamily. However, I understand that the inheritance from AbstractFamily makes the bases of FiniteRankFreeModule's more similar to those of other free modules in Sage, which is certainly a good thing => positive review from my side.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 27, 2022

comment:25

Thank you!

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 28, 2022

comment:26

(commented on the wrong ticket)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 28, 2022

Dependencies: #31276

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 28, 2022

Changed dependencies from #31276 to none

@vbraun
Copy link
Member

vbraun commented Aug 29, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants