Opened 4 years ago
Closed 4 years ago
#23703 closed defect (fixed)
free_quadratic_module intersection returns wrong result
Reported by:  sbrandhorst  Owned by:  

Priority:  major  Milestone:  sage8.1 
Component:  linear algebra  Keywords:  sd91 
Cc:  Merged in:  
Authors:  Simon Brandhorst  Reviewers:  Frédéric Chapoton 
Report Upstream:  N/A  Work issues:  
Branch:  2e4138f (Commits, GitHub, GitLab)  Commit:  2e4138f03c136a2824d3164d5dcd6289687e25c4 
Dependencies:  Stopgaps: 
Description (last modified by )
The first intersection is good. The second is nonsense.
sage: A=FreeQuadraticModule(ZZ,1,matrix.identity(1)) sage: B=A.span([[1/2]]) sage: C=B.span([[1]]) sage: B.intersection(C) Free module of degree 1 and rank 1 over Integer Ring Echelon basis matrix: [1] sage: C.intersection(B) Free module of degree 1 and rank 1 over Integer Ring Echelon basis matrix: [1/2]
It must be the free_quadratic_module code as the free module stuff seems to work
sage: A=FreeModule(ZZ,1) sage: B=A.span([[1/2]]) sage: C=B.span([[1]]) sage: C.intersection(B) Free module of degree 1 and rank 1 over Integer Ring Echelon basis matrix: [1] sage: B.intersection(C) Free module of degree 1 and rank 1 over Integer Ring Echelon basis matrix: [1]
This also breaks functions using intersections e.g. module homomorphisms of quotients.
Change History (18)
comment:1 Changed 4 years ago by
 Keywords sd91 added; abelian group quotient homomorphism removed
comment:2 Changed 4 years ago by
 Description modified (diff)
 Summary changed from Homorphism of finitely generated module over PIDs not working to free_quadratic_module intersection returns wrong result
comment:3 Changed 4 years ago by
comment:4 Changed 4 years ago by
beware of #23646
comment:5 Changed 4 years ago by
 Branch set to u/sbrandhorst/free_quadratic_module_intersection_returns_wrong_result
comment:6 Changed 4 years ago by
 Commit set to ef8cf1f67a4791231ba1242af38a5030d6e4500c
 Status changed from new to needs_review
New commits:
ef8cf1f  Removed the __richcmp__ methods. If they are just copy pasted from free_module.py, we may inherit the methods just as well.

comment:7 Changed 4 years ago by
You should keep all the doctests somewhere.
comment:8 Changed 4 years ago by
I just checked: the old doctests did not even call the richcmp methods defined. So I will modify them old doctests.
Btw in any case (old&new) we get:
sage: FreeQuadraticModule(ZZ,1,matrix.identity(1))==FreeQuadraticModule(ZZ,1,matrix.identity(1)*2) True
Is that a bug or a feature? I would consider two quadratic spaces with different inner products very different and mathematically not equal. After all the inner product is the whole point of this class.
comment:9 Changed 4 years ago by
 Commit changed from ef8cf1f67a4791231ba1242af38a5030d6e4500c to d6e68517b6786be74564d731cca1761f1972ace4
Branch pushed to git repo; I updated commit sha1. New commits:
d6e6851  Added doctests for comparison of quadratic modules.

comment:10 Changed 4 years ago by
wrong syntax for the new doc..
when a line end with ::, it must be followd by an empty line. And the next lines must be indented by 4 spaces.
comment:11 Changed 4 years ago by
 Commit changed from d6e68517b6786be74564d731cca1761f1972ace4 to de9d26fa65f6118db7d959b605782b0fe9296383
Branch pushed to git repo; I updated commit sha1. New commits:
de9d26f  Fix syntax for new doc

comment:12 Changed 4 years ago by
not fixed :
+ EXAMPLES:: + + We compare rank three free modules over the integers and + rationals:: + sage: Q3 = FreeQuadraticModule(QQ,3,matrix.identity(3))
comment:13 Changed 4 years ago by
 Commit changed from de9d26fa65f6118db7d959b605782b0fe9296383 to d82b285c01c7e9c46baa6db128fa6e6f272a0333
Branch pushed to git repo; I updated commit sha1. New commits:
d82b285  Another doc syntax fix.

comment:14 Changed 4 years ago by
 Commit changed from d82b285c01c7e9c46baa6db128fa6e6f272a0333 to f822934ca67c9a59a2cf082fbe66d9c6bfd08a03
Branch pushed to git repo; I updated commit sha1. New commits:
f822934  Hopefully the final documentation syntax fix...

comment:15 Changed 4 years ago by
 Commit changed from f822934ca67c9a59a2cf082fbe66d9c6bfd08a03 to 2e4138f03c136a2824d3164d5dcd6289687e25c4
Branch pushed to git repo; I updated commit sha1. New commits:
2e4138f  labeled the comparison docs as tests

comment:16 Changed 4 years ago by
Doctests pass and the html documentation builds (at least the free_quadratic_module part).
comment:17 Changed 4 years ago by
 Reviewers set to Frédéric Chapoton
 Status changed from needs_review to positive_review
ok then.
comment:18 Changed 4 years ago by
 Branch changed from u/sbrandhorst/free_quadratic_module_intersection_returns_wrong_result to 2e4138f03c136a2824d3164d5dcd6289687e25c4
 Resolution set to fixed
 Status changed from positive_review to closed
The story continues ... maybe this is the real bug?