Opened 3 years ago
Closed 3 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:  sage8.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 nondegenerate 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 ZZbasis (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 3 years ago by
 Branch set to u/sbrandhorst/a_base_class_for_integral_quadratic_forms_seen_as_modules_with_a_bilinear_form_
comment:2 Changed 3 years ago by
 Commit set to 8b7671388303bb52dc30f3c94b80f4474560f424
comment:3 Changed 3 years ago by
 Status changed from new to needs_review
comment:4 Changed 3 years ago by
 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 3 years ago by
 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 3 years ago by
 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 3 years ago by
 Commit changed from 8b7671388303bb52dc30f3c94b80f4474560f424 to e58f4cefcb860b93a8a50a644f46c196254709d3
Branch pushed to git repo; I updated commit sha1. New commits:
e58f4ce  Doctest formatting.

comment:8 Changed 3 years ago by
 Dependencies set to #22720
comment:9 Changed 3 years ago by
 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 3 years ago by
 Commit changed from e58f4cefcb860b93a8a50a644f46c196254709d3 to 45d83e89d909983a37ad582e8e1331e56ca9419d
comment:11 Changed 3 years ago by
I'm happy with it; Simon, you should take a look at my changes.
comment:12 Changed 3 years ago by
 Commit changed from 45d83e89d909983a37ad582e8e1331e56ca9419d to 09a7e7debacf409eb618a1fa8f7e5b0142901edf
Branch pushed to git repo; I updated commit sha1. New commits:
09a7e7d  Remove prime_to_m_part import

comment:14 Changed 3 years ago by
 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:
69955c7  Fix case for math directive

comment:15 Changed 3 years ago by
 Dependencies changed from #22720 to #22720,#23703
comment:16 Changed 3 years ago by
 Status changed from needs_review to positive_review
All doctests pass & the documentation builds.
comment:17 Changed 3 years ago by
 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 3 years ago by
 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:
abef20e  Doctests.

comment:19 Changed 3 years ago by
 Status changed from needs_review to positive_review
Thanks for fixing those!
comment:20 Changed 3 years ago by
 Status changed from positive_review to needs_work
The documentation for IntegralLattice? is wrong.
comment:21 Changed 3 years ago by
 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 3 years ago by
 Commit changed from abef20e14cbf24e882704d84dcc950439301544a to 12acb67fbaea03e6f9420248d3458b05811fe2b3
Branch pushed to git repo; I updated commit sha1. New commits:
12acb67  Update documentation and formatting. Add TODOs

comment:23 Changed 3 years ago by
 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 3 years ago by
 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 3 years ago by
 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 3 years ago by
 Commit changed from 12acb67fbaea03e6f9420248d3458b05811fe2b3 to 74a7393c87ceb7f1d6061bc4e1eb9c6efc88ae2d
Branch pushed to git repo; I updated commit sha1. New commits:
74a7393  Fixed a syntax error and added a doctest for orthogonal_complement.

comment:27 Changed 3 years ago by
 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 3 years ago by
 Commit changed from 74a7393c87ceb7f1d6061bc4e1eb9c6efc88ae2d to c96014b267046a9b36d7f6d5612b5776485c6076
Positive review if you're happy with my changes.
New commits:
c96014b  Editing docstrings, a few small code fixes

comment:29 Changed 3 years ago by
 Status changed from needs_review to positive_review
comment:30 Changed 3 years ago by
 Status changed from positive_review to needs_work
sage t long warnlong 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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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/sitepackages/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 warnlong 64.6 src/sage/modules/free_quadratic_module_integer_symmetric.py # 2 doctests failed 
comment:31 Changed 3 years ago by
I cannot reproduce this :(. Can someone else?
./sage t long warnlong 64.6 src/sage/modules/free_quadratic_module_integer_symmetric.py Running doctests with ID 201710051126067db5d712. 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 warnlong 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 3 years ago by
I get the failures when I test it with #23915.
comment:33 Changed 3 years ago by
 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 3 years ago by
 Commit changed from c96014b267046a9b36d7f6d5612b5776485c6076 to d727d6c46bb3b74e4e87f122e9d3da3856897adc
Branch pushed to git repo; I updated commit sha1. New commits:
d727d6c  Fixed orthogonal_complement()

comment:35 Changed 3 years ago by
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)) nondeterministic? This bug confuses me.
comment:36 Changed 3 years ago by
 Commit changed from d727d6c46bb3b74e4e87f122e9d3da3856897adc to 06b8167770e2cf4d025b7d1e444d9878542f0b1f
Branch pushed to git repo; I updated commit sha1. New commits:
06b8167  Fixed 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 3 years ago by
 Status changed from needs_work to needs_review
Lets see what the patchbot says.
comment:38 Changed 3 years ago by
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 3 years ago by
 Commit changed from 06b8167770e2cf4d025b7d1e444d9878542f0b1f to 707a74e27dd6fea52d3785fe4dbe1f46b61d3a84
Branch pushed to git repo; I updated commit sha1. New commits:
707a74e  clean up in __richcmp__ for free modules.

comment:40 Changed 3 years ago by
comment:41 Changed 3 years ago by
 Status changed from needs_review to positive_review
Looks good to me. The failure in geometry/cone.py
seems to have resulted from an outofmemory problem.
comment:42 Changed 3 years ago by
 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
Branch pushed to git repo; I updated commit sha1. New commits:
Doctests added, some bugs fixed