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: sage-8.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:

Status badges

Description (last modified by sbrandhorst)

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 sbrandhorst

  • Keywords sd91 added; abelian group quotient homomorphism removed

comment:2 Changed 4 years ago by sbrandhorst

  • 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 sbrandhorst

The story continues ... maybe this is the real bug? 

sage: A=FreeQuadraticModule(ZZ,1,matrix.identity(1))
sage: B=A.span([[1]])
sage: B==B.ambient_vector_space()
True
sage: B=A.span([[1/2]])
sage: B==B.ambient_vector_space()
False

comment:4 Changed 4 years ago by chapoton

beware of #23646

comment:5 Changed 4 years ago by sbrandhorst

  • Branch set to u/sbrandhorst/free_quadratic_module_intersection_returns_wrong_result

comment:6 Changed 4 years ago by sbrandhorst

  • Commit set to ef8cf1f67a4791231ba1242af38a5030d6e4500c
  • Status changed from new to needs_review

New commits:

​ef8cf1fRemoved 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 chapoton

You should keep all the doctests somewhere.

comment:8 Changed 4 years ago by sbrandhorst

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 git

  • Commit changed from ef8cf1f67a4791231ba1242af38a5030d6e4500c to d6e68517b6786be74564d731cca1761f1972ace4

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

​d6e6851Added doctests for comparison of quadratic modules.

comment:10 Changed 4 years ago by chapoton

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 git

  • Commit changed from d6e68517b6786be74564d731cca1761f1972ace4 to de9d26fa65f6118db7d959b605782b0fe9296383

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

​de9d26fFix syntax for new doc

comment:12 Changed 4 years ago by chapoton

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 git

  • Commit changed from de9d26fa65f6118db7d959b605782b0fe9296383 to d82b285c01c7e9c46baa6db128fa6e6f272a0333

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

​d82b285Another doc syntax fix.

comment:14 Changed 4 years ago by git

  • Commit changed from d82b285c01c7e9c46baa6db128fa6e6f272a0333 to f822934ca67c9a59a2cf082fbe66d9c6bfd08a03

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

​f822934Hopefully the final documentation syntax fix...

comment:15 Changed 4 years ago by git

  • Commit changed from f822934ca67c9a59a2cf082fbe66d9c6bfd08a03 to 2e4138f03c136a2824d3164d5dcd6289687e25c4

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

​2e4138flabeled the comparison docs as tests

comment:16 Changed 4 years ago by sbrandhorst

Doctests pass and the html documentation builds (at least the free_quadratic_module part).

comment:17 Changed 4 years ago by chapoton

  • Reviewers set to Frédéric Chapoton
  • Status changed from needs_review to positive_review

ok then.

comment:18 Changed 4 years ago by vbraun

  • 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
Note: See TracTickets for help on using tickets.