Orthogonal groups for IntegralLattices

simonbrandhorst commented 7 years ago

Implement a class for the orthogonal group of integral lattices. Its elements are isometries of the ambient spaces which preserve the lattice.

As functionality:

Its elements have an action on elements of the ambient space and the lattice
action on the dual lattice
It is a matrix group
Compute generators in case the lattice is positive definite. One can use QuadraticForm.automorphisms() for that.
It should also work for indefinite lattices. Just have some not implemented errors - like the generators.
Allow subgroups of the orthogonal group.

Depends on #23634 Depends on #23699 Depends on #24150 Depends on #24702

Component: linear algebra

Author: Simon Brandhorst

Branch/Commit: 5ae4594

Reviewer: David Roe

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

simonbrandhorst commented 7 years ago

Description changed:

--- 
+++ 
@@ -4,6 +4,7 @@
 As functionality:
 - Its elements have an action on elements of the ambient space and the lattice
 - action on the dual lattice
-- It is be a matrix group
+- It is a matrix group
 - Compute generators in case the lattice is positive definite. One can use `QuadraticForm.automorphisms()` for that.
 - It should also work for indefinite lattices. Just have some not implemented errors - like the generators.
+- Allow subgroups of the orthogonal group.

simonbrandhorst commented 6 years ago

Branch: u/sbrandhorst/orthogonal_groups_for_integrallattices

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

Commit: 3d4146f

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

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

3d4146f Formating.

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

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

63a5914 Set category to none if we do not know.

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

Changed commit from 3d4146f to 63a5914

simonbrandhorst commented 6 years ago

comment:6

We still need to make sure that the actions are working. In particular on the discriminantGroup.

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

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

`cb1890f`	`Merge branch 'develop' into t/24036/orthogonal_groups_for_integrallattices`
`6f6589f`	`Group action on submodules`

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

Changed commit from 63a5914 to 6f6589f

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

Changed commit from 6f6589f to 4cb13ca

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

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

`e5cecc5`	`Polished docstrings`
`38091bb`	`Fixed indentation for docstrings at module level.`
`a7a9c39`	`Lazy import IntegralLattice`
`6ca8126`	`Formatting, minor doc changes, cleanup, and whitespace removal.`
`76987fa`	`Some trivial changes in the documentation.`
`12c8163`	`Added b as an alias for _mul_.`
`c8ce05b`	Added documentation for `b` and `inner_product`
`4569a08`	`Fixed doctest failures from changed printing.`
`df6a6ae`	`Merge branch 't/23699/torsion_quadratic_module_symmetric' into t/24036/orthogonal_groups_for_integrallattices`
`4cb13ca`	`Use the new factory function for doctests`

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

Changed commit from 4cb13ca to ff474a5

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

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

`8e983d0`	`Merge branch 'public/lattices/torsion_quadratic_module_symmetric-23699' of git://trac.sagemath.org/sage into t/24150/factory_function_for_famous_lattices`
`9337620`	`Use the new factory function in the documentation of the other methods and clean up.`
`e0057df`	`remove a superflous blank line`
`27084a9`	`Reducing lines to <80 characters`
`94757dd`	`Fixed a typo. Doctests pass now.`
`63dc485`	`a few extra spaces after a comma`
`220f941`	`Some more small docfixes`
`d4cf39f`	`Doing some code cleaning, improvements, and tweaks.`
`d1807f1`	`Merge branch 'public/quadratic_forms/lattice_factory-24150' of git://trac.sagemath.org/sage into t/24036/orthogonal_groups_for_integrallattices`
`ff474a5`	`Added an action on the discriminant group`

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

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

0fb495a Documentation fixes

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

Changed commit from ff474a5 to 0fb495a

simonbrandhorst commented 6 years ago

Changed dependencies from #23634 to #23634,#23699,#24150

simonbrandhorst commented 6 years ago

Author: Simon Brandhorst

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

Changed commit from 0fb495a to dab23be

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

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

dab23be Merge branch 'develop' into t/24036/orthogonal_groups_for_integrallattices

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

Changed commit from dab23be to 1569ee1

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

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

1569ee1 Merge branch 'develop' into t/24036/orthogonal_groups_for_integrallattices

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

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

1f60808 Improved documentation.

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

Changed commit from 1569ee1 to 1f60808

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

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

e7c98fb Some extra spaces.

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

Changed commit from 1f60808 to e7c98fb

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

Changed commit from e7c98fb to 0a39a03

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

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

0a39a03 A few more docfixes

simonbrandhorst commented 6 years ago

comment:17

The documentation builds and seems to look O.K. Tests pass too. Still waiting for patchbot.

One might want to consider moving the code to a different file to improve documentation?

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

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

ee1c7c9 Moved code to a new file to improve doc.

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

Changed commit from 0a39a03 to ee1c7c9

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

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

38ac155 Fixing bugs

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

Changed commit from ee1c7c9 to 38ac155

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

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

e4cb146 Merge branch 'develop' of git://trac.sagemath.org/sage into t/24036/orthogonal_groups_for_integrallattices

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

Changed commit from 38ac155 to e4cb146

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

Changed commit from e4cb146 to 480fd6e

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

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

`92f85fa`	`Action on submodules of the invariant_quotient_module`
`9bf3e7b`	`Merge branch 'u/sbrandhorst/orthogonal_groups_for_integrallattices' of git://trac.sagemath.org/sage into t/24036/orthogonal_groups_for_integrallattices`
`480fd6e`	`Merge branch 'develop' into t/24036/orthogonal_groups_for_integrallattices`

simonbrandhorst commented 6 years ago

Changed dependencies from #23634,#23699,#24150 to #23634,#23699,#24150,#24702

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

Changed commit from 480fd6e to bb637b5

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

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

`f341138`	`Fix free_module.intersection`
`8759cb1`	`Fix free_module.saturation`
`0dd7769`	`Indentation`
`bb637b5`	`Merge branch 'u/sbrandhorst/intersections_of_freequadraticmodules_have_the_wrong_ambient_module' of git://trac.sagemath.org/sage into t/24036/orthogonal_groups_for_integrallattices`

roed314 commented 6 years ago

Reviewer: David Roe

roed314 commented 6 years ago

comment:24

Some comment:

The line submodule -- an invariant submodule in GroupActionOnQuotientModule needs to be changed from submodule to quotient.
The actions allow is_left=True but then ignore it in the implementation of _call_.
There is no check that the invariant submodule and quotient module passed into the __init__ method of GroupOfIsometries are actually invariant.
In /src/sage/groups/matrix_gps/orthogonal.py, you say that you add OrthogonalMatrixGroup_with_gap but don't, and you import FinitelyGeneratedMatrixGroup_gap and Action but don't use them.
It's better to use gens is None rather than gens == None
The is_finite keyword on the orthogonal_group method should be documented better (or removed).
Should orthogonal_group be a cached method?

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

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

fbb38c5 left actions & cleanup

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

Changed commit from bb637b5 to fbb38c5

simonbrandhorst commented 6 years ago

comment:26

You wrote: Should orthogonal_group be a cached method?

I would like that. But sometimes the input (=a list of generators) may be non-hashable and then @cashed method throws an error. Is there an easy way around?

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

Changed commit from fbb38c5 to 5ae4594

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

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

5ae4594 Indentation.

simonbrandhorst commented 6 years ago

comment:29

Doc builds now.

roed314 commented 6 years ago

comment:30

Replying to @simonbrandhorst:

You wrote: Should orthogonal_group be a cached method?

I would like that. But sometimes the input (=a list of generators) may be non-hashable and then @cached_method throws an error. Is there an easy way around?

Yeah, @cached_method supports a key to normalize the input, which you can use to make it hashable. From the docstring for cached_method,

    The parameter ``key`` can be used to pass a function which creates a
    custom cache key for inputs. In the following example, this parameter is
    used to ignore the ``algorithm`` keyword for caching::

        sage: class A(object):
        ....:     def _f_normalize(self, x, algorithm): return x
        ....:     @cached_method(key=_f_normalize)
        ....:     def f(self, x, algorithm='default'): return x
        sage: a = A()
        sage: a.f(1, algorithm="default") is a.f(1) is a.f(1, algorithm="algorithm")
        True

roed314 commented 6 years ago

comment:31

But you can leave that for another ticket. This looks good to me.

vbraun commented 6 years ago

Changed branch from u/sbrandhorst/orthogonal_groups_for_integrallattices to 5ae4594

sagemath / sage

Orthogonal groups for IntegralLattices #24036