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

Affine and Euclidean groups #14015

Closed
vbraun opened this issue Jan 25, 2013 · 15 comments
Closed

Affine and Euclidean groups #14015

vbraun opened this issue Jan 25, 2013 · 15 comments

Comments

@vbraun
Copy link
Member

vbraun commented Jan 25, 2013

This ticket implements basic affine groups and Euclidean groups:

sage: G = AffineGroup(3, QQ)
sage: g = G.random_element(); g
     [   2 -1/2    0]     [  0]
x|-> [   1   -1   -1] x + [-32]
     [   0   -2   -2]     [1/3]
sage: g*g
     [ 7/2 -1/2  1/2]     [   16]
x|-> [   1  5/2    3] x + [ -1/3]
     [  -2    6    6]     [191/3]
sage: g*g.inverse()
     [1 0 0]     [0]
x|-> [0 1 0] x + [0]
     [0 0 1]     [0]

Apply:

Depends on #14040
Depends on #14014

Component: group theory

Author: Volker Braun, Travis Scrimshaw

Reviewer: Travis Scrimshaw, Volker Braun

Merged: sage-5.11.beta1

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

@vbraun
Copy link
Member Author

vbraun commented Jan 25, 2013

Initial patch

@vbraun

This comment has been minimized.

@vbraun
Copy link
Member Author

vbraun commented Jan 25, 2013

comment:1

Attachment: trac_14015_affine_group.patch.gz

@vbraun
Copy link
Member Author

vbraun commented Feb 12, 2013

Dependencies: #14040, #14014

@vbraun
Copy link
Member Author

vbraun commented Feb 12, 2013

comment:2

To be rebased on top of #14040, #14014.

@tscrim
Copy link
Collaborator

tscrim commented Jun 3, 2013

Attachment: trac_14015-affine_groups-ts.patch.gz

@tscrim
Copy link
Collaborator

tscrim commented Jun 3, 2013

comment:3

Hey Volker,

Here's a rebased version with my review changes. I've removed the need for the *_generic classes and made the docstrings be at the class level so that they are visible using introspection. I've also added a method to get the lifted matrix space (representation of affine transformations as linear transformations) as linear_space(), and made a few docstring tweaks. If you're happy with my changes, you can set this to positive review.

Best,

Travis

For patchbot:

Apply: trac_14015-affine_groups-ts.patch

@tscrim
Copy link
Collaborator

tscrim commented Jun 3, 2013

Reviewer: Travis Scrimshaw

@tscrim

This comment has been minimized.

@vbraun
Copy link
Member Author

vbraun commented Jun 3, 2013

comment:4

The _generic suffix was there so that we can later also wrap GAP's affine groups (especially for finite fields)

@tscrim
Copy link
Collaborator

tscrim commented Jun 5, 2013

comment:5

You can have the GAP's affine groups and have the __classcall__() return that class (see sage.combinat.partition.Partitions or sage.combinat.tableau.Tableaux as more complete/complicated examples). IMO this is cleaner since the we the class doesn't have any extra qualifiers, the (single) entry point matches the (base) class, and the classes have the correct naming scheme. Thus it is still extendable.

If the input format needs to be changed and exposed to the global namespace, you can implement a __classcall__() on the GAP wrapper parent (and likely the input will still need to standardized).

@vbraun
Copy link
Member Author

vbraun commented Jun 5, 2013

comment:6

And you need some way to circumvent the enforced argument normalization for internal use where you know that the arguments don't have to be normalized. In terms of complexity / lines of code, I think its pretty much a draw. Which is to say, you end up using a lot of complicated machinery for no real advantage. And it gets even more complicated if you start deriving the class. And it breaks the symmetry between different implementations. If I had seen a real advantage with the __classcall__ mechanism then I would have used it myself.

@vbraun
Copy link
Member Author

vbraun commented Jun 5, 2013

Changed author from Volker Braun to Volker Braun, Travis Scrimshaw

@vbraun
Copy link
Member Author

vbraun commented Jun 5, 2013

Changed reviewer from Travis Scrimshaw to Travis Scrimshaw, Volker Braun

@jdemeyer
Copy link

Merged: sage-5.11.beta1

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

4 participants