Opened 2 years ago

Closed 2 years ago

#23634 closed enhancement (fixed)

A base class for integral quadratic forms seen as modules with a bilinear form.

Reported by: sbrandhorst Owned by:
Priority: major Milestone: sage-8.1
Component: linear algebra Keywords: sd91
Cc: Merged in:
Authors: Simon Brandhorst Reviewers: David Roe
Report Upstream: N/A Work issues:
Branch: 707a74e (Commits) Commit: 707a74e27dd6fea52d3785fe4dbe1f46b61d3a84
Dependencies: #22720,#23703 Stopgaps:

Description

An integral lattice is a free abelian group L together with a non-degenerate integer valued bilinear form L x L --> ZZ.

In sage a lattice should consist of:

  • an ambient QQ vector space with an inner product matrix
  • a distinguished ZZ-basis (with possibly rational entries)

Integral lattices and quadratic forms are mathematically basically the same. For me the difference in thinking is that lattices live in an ambient space. Thus, one can have several lattices in an ambient space and compare their mutual relations (containment, embedding etc.)

Probably it should be derived from sage.modules.free_quadratic_module.FreeQuadraticModule_submodule_with_basis_pi

New methods should include

  • discriminant groups and forms
  • over and sublattices
  • Orthogonal groups
  • direct sums
  • is_even
  • is_primitive sublattice
  • orthogonal complements
  • the dual "lattice"

Change History (42)

comment:1 Changed 2 years ago by sbrandhorst

  • Branch set to u/sbrandhorst/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_

comment:2 Changed 2 years ago by git

  • Commit set to 8b7671388303bb52dc30f3c94b80f4474560f424

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

8b76713Doctests added, some bugs fixed

comment:3 Changed 2 years ago by sbrandhorst

  • Status changed from new to needs_review

comment:4 Changed 2 years ago by roed

  • Reviewers set to David Roe
  • Status changed from needs_review to needs_work

Since lattices are parents, they should be unique (for given defining data). I would suggest using sage.structure.factory.Factory, but you may be able to get away with sage.structure.unique_representation.UniqueRepresentation instead.

comment:5 Changed 2 years ago by sbrandhorst

  • Status changed from needs_work to needs_info

The very similar classes of submodules

sage.modules.free_module.FreeModule_submodule_field sage.modules.free_module.FreeModule_submodule_pid sage.modules.free_module_integer.FreeModule_submodule_with_basis_integer sage.modules.free_quadratic_module.FreeQuadraticModule_submodule_pid

are not unique. In my view lattices are submodules of an ambient space (with certain restrictions), too. Can you make a case why this class should be unique while they are not? Note that our lattices are equipped with a distinguished basis. This could change during the runtime. This seems to conflict with the uniqueness of the objects?

comment:6 Changed 2 years ago by roed

  • Status changed from needs_info to needs_review

Okay. I'll look at the code again. There's some trouble with an internal server error, so I'm trying to see if a comment changes anything.

comment:7 Changed 2 years ago by git

  • Commit changed from 8b7671388303bb52dc30f3c94b80f4474560f424 to e58f4cefcb860b93a8a50a644f46c196254709d3

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

e58f4ceDoctest formatting.

comment:8 Changed 2 years ago by roed

  • Dependencies set to #22720

comment:9 Changed 2 years ago by roed

  • Branch changed from u/sbrandhorst/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_ to u/roed/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_

comment:10 Changed 2 years ago by git

  • Commit changed from e58f4cefcb860b93a8a50a644f46c196254709d3 to 45d83e89d909983a37ad582e8e1331e56ca9419d

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

2b7ba4bchange annihilator when cardinality is 1
1013f56fixed 0 to 1
360ce18added documentation
45d83e8Merge branch 'u/amy/additiveabeliangroup_____annihilator___fails_' of git://trac.sagemath.org/sage into t/23634/integral_qforms

comment:11 Changed 2 years ago by roed

I'm happy with it; Simon, you should take a look at my changes.

comment:12 Changed 2 years ago by git

  • Commit changed from 45d83e89d909983a37ad582e8e1331e56ca9419d to 09a7e7debacf409eb618a1fa8f7e5b0142901edf

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

09a7e7dRemove prime_to_m_part import

comment:13 Changed 2 years ago by sbrandhorst

  • Status changed from needs_review to positive_review

Looks good!

comment:14 Changed 2 years ago by git

  • Commit changed from 09a7e7debacf409eb618a1fa8f7e5b0142901edf to 69955c7fa3c2d348b87cd7f0c803a45d453a8312
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

69955c7Fix case for math directive

comment:15 Changed 2 years ago by sbrandhorst

  • Dependencies changed from #22720 to #22720,#23703

comment:16 Changed 2 years ago by sbrandhorst

  • Status changed from needs_review to positive_review

All doctests pass & the documentation builds.

comment:17 Changed 2 years ago by sbrandhorst

  • Branch changed from u/roed/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_ to u/sbrandhorst/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_

comment:18 Changed 2 years ago by git

  • Commit changed from 69955c7fa3c2d348b87cd7f0c803a45d453a8312 to abef20e14cbf24e882704d84dcc950439301544a
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

abef20eDoctests.

comment:19 Changed 2 years ago by roed

  • Status changed from needs_review to positive_review

Thanks for fixing those!

comment:20 Changed 2 years ago by sbrandhorst

  • Status changed from positive_review to needs_work

The documentation for IntegralLattice? is wrong.

comment:21 Changed 2 years ago by amy

  • Branch changed from u/sbrandhorst/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_ to u/amy/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_

comment:22 Changed 2 years ago by git

  • Commit changed from abef20e14cbf24e882704d84dcc950439301544a to 12acb67fbaea03e6f9420248d3458b05811fe2b3

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

12acb67Update documentation and formatting. Add TODOs

comment:23 Changed 2 years ago by sbrandhorst

  • Branch changed from u/amy/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_ to u/sbrandhorst/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_

comment:24 Changed 2 years ago by sbrandhorst

  • Branch changed from u/sbrandhorst/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_ to u/amy/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_
  • Status changed from needs_work to needs_review

comment:25 Changed 2 years ago by sbrandhorst

  • Branch changed from u/amy/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_ to u/sbrandhorst/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_

comment:26 Changed 2 years ago by git

  • Commit changed from 12acb67fbaea03e6f9420248d3458b05811fe2b3 to 74a7393c87ceb7f1d6061bc4e1eb9c6efc88ae2d

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

74a7393Fixed a syntax error and added a doctest for orthogonal_complement.

comment:27 Changed 2 years ago by roed

  • Branch changed from u/sbrandhorst/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_ to u/roed/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_

comment:28 Changed 2 years ago by roed

  • Commit changed from 74a7393c87ceb7f1d6061bc4e1eb9c6efc88ae2d to c96014b267046a9b36d7f6d5612b5776485c6076

Positive review if you're happy with my changes.


New commits:

c96014bEditing docstrings, a few small code fixes

comment:29 Changed 2 years ago by sbrandhorst

  • Status changed from needs_review to positive_review

comment:30 Changed 2 years ago by vbraun

  • Status changed from positive_review to needs_work
sage -t --long --warn-long 64.6 src/sage/modules/free_quadratic_module_integer_symmetric.py
**********************************************************************
File "src/sage/modules/free_quadratic_module_integer_symmetric.py", line 112, in sage.modules.free_quadratic_module_integer_symmetric.FreeQuadraticModule_integer_symmetric.__init__
Failed example:
    TestSuite(L).run()
Expected nothing
Got:
      Failure in _test_pickling:
      Traceback (most recent call last):
        File "/mnt/disk/home/release/Sage/local/lib/python2.7/site-packages/sage/misc/sage_unittest.py", line 294, in run
          test_method(tester = tester)
        File "sage/structure/sage_object.pyx", line 683, in sage.structure.sage_object.SageObject._test_pickling (build/cythonized/sage/structure/sage_object.c:5952)
          tester.assertEqual(loads(dumps(self)), self)
        File "/mnt/disk/home/release/Sage/local/lib/python2.7/unittest/case.py", line 513, in assertEqual
          assertion_func(first, second, msg=msg)
        File "/mnt/disk/home/release/Sage/local/lib/python2.7/unittest/case.py", line 506, in _baseAssertEqual
          raise self.failureException(msg)
      AssertionError: (1, 0) != (1, 0)
      ------------------------------------------------------------
      The following tests failed: _test_pickling
    Failure in _test_elements
    The following tests failed: _test_elements
**********************************************************************
File "src/sage/modules/free_quadratic_module_integer_symmetric.py", line 340, in sage.modules.free_quadratic_module_integer_symmetric.FreeQuadraticModule_integer_symmetric.orthogonal_complement
Failed example:
    L.orthogonal_complement(S)
Exception raised:
    Traceback (most recent call last):
      File "/mnt/disk/home/release/Sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 515, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/mnt/disk/home/release/Sage/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 885, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.modules.free_quadratic_module_integer_symmetric.FreeQuadraticModule_integer_symmetric.orthogonal_complement[3]>", line 1, in <module>
        L.orthogonal_complement(S)
      File "/mnt/disk/home/release/Sage/local/lib/python2.7/site-packages/sage/modules/free_quadratic_module_integer_symmetric.py", line 365, in orthogonal_complement
        return self.sublattice(self.intersection(K).basis())
      File "/mnt/disk/home/release/Sage/local/lib/python2.7/site-packages/sage/modules/free_module.py", line 2550, in intersection
        raise ArithmeticError("self and other must be embedded in the same ambient space.")
    ArithmeticError: self and other must be embedded in the same ambient space.
**********************************************************************
2 items had failures:
   1 of   4 in sage.modules.free_quadratic_module_integer_symmetric.FreeQuadraticModule_integer_symmetric.__init__
   1 of   7 in sage.modules.free_quadratic_module_integer_symmetric.FreeQuadraticModule_integer_symmetric.orthogonal_complement
    [65 tests, 2 failures, 0.64 s]
----------------------------------------------------------------------
sage -t --long --warn-long 64.6 src/sage/modules/free_quadratic_module_integer_symmetric.py  # 2 doctests failed
----------------------------------------------------------------------

comment:31 Changed 2 years ago by sbrandhorst

I cannot reproduce this :-(. Can someone else?

./sage -t --long --warn-long 64.6 src/sage/modules/free_quadratic_module_integer_symmetric.py
Running doctests with ID 2017-10-05-11-26-06-7db5d712.
Git branch: t/23634/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_
Using --optional=atlas,ccache,mpir,python2,sage
Doctesting 1 file.
sage -t --long --warn-long 64.6 src/sage/modules/free_quadratic_module_integer_symmetric.py
    [65 tests, 1.21 s]
----------------------------------------------------------------------
All tests passed!
----------------------------------------------------------------------
Total time for all tests: 1.3 seconds
    cpu time: 0.9 seconds
    cumulative wall time: 1.2 seconds

comment:32 Changed 2 years ago by tscrim

I get the failures when I test it with #23915.

comment:33 Changed 2 years ago by sbrandhorst

  • Branch changed from u/roed/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_ to u/sbrandhorst/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_

comment:34 Changed 2 years ago by git

  • Commit changed from c96014b267046a9b36d7f6d5612b5776485c6076 to d727d6c46bb3b74e4e87f122e9d3da3856897adc

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

d727d6cFixed orthogonal_complement()

comment:35 Changed 2 years ago by sbrandhorst

sage: from sage.modules.free_quadratic_module_integer_symmetric import *
sage: L=IntegralLattice(Matrix(ZZ,2,2,[0,1,1,0]))
sage: x=L.an_element()
sage: x == loads(dumps(x))
True
sage: x == loads(dumps(x))
False
sage: x == loads(dumps(x))
True
sage: x == loads(dumps(x))
True

Uhm. Is loads(dumps(x)) non-deterministic? This bug confuses me.

comment:36 Changed 2 years ago by git

  • Commit changed from d727d6c46bb3b74e4e87f122e9d3da3856897adc to 06b8167770e2cf4d025b7d1e444d9878542f0b1f

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

06b8167Fixed the loads(dumps(x)) issue by fixing comparison for free modules. The change only kicks in if the unique parent assumption is violated. This seems to happen in the loads dumps process.

comment:37 Changed 2 years ago by sbrandhorst

  • Status changed from needs_work to needs_review

Lets see what the patchbot says.

comment:38 Changed 2 years ago by tscrim

Why not just use richcmp(lx, rx, op)?

Also, instead of

self._inner_product_is_dot_product()==True

you should just do self._inner_product_is_dot_product().

comment:39 Changed 2 years ago by git

  • Commit changed from 06b8167770e2cf4d025b7d1e444d9878542f0b1f to 707a74e27dd6fea52d3785fe4dbe1f46b61d3a84

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

707a74eclean up in __richcmp__ for free modules.

comment:40 Changed 2 years ago by sbrandhorst

Good point. Changed that.


New commits:

707a74eclean up in __richcmp__ for free modules.

comment:41 Changed 2 years ago by roed

  • Status changed from needs_review to positive_review

Looks good to me. The failure in geometry/cone.py seems to have resulted from an out-of-memory problem.

comment:42 Changed 2 years ago by vbraun

  • Branch changed from u/sbrandhorst/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_ to 707a74e27dd6fea52d3785fe4dbe1f46b61d3a84
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.