sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.43k stars 478 forks source link

vectors in IntegralLattice compute norm incorrectly #38543

Open yyyyx4 opened 2 months ago

yyyyx4 commented 2 months ago

From here:

sage: L = IntegralLattice(matrix([[1000,0],[0,1]]))
sage: v = L.0; v
(1, 0)
sage: v.parent()
Lattice of degree 2 and rank 2 over Integer Ring
Standard basis
Inner product matrix:
[1000    0]
[   0    1]
sage: v.norm()
1
sage: v.inner_product(v)
1000

The .parent() is set to the lattice in question, and .inner_product() behaves as expected, but the .norm() computes the standard Euclidean norm. This inconsistency causes confusion and bugs.

Cc: @grhkm21

grhkm21 commented 2 months ago

Hi. Yes, that. Also v.inner_product(v) gives the last line (1000).

yyyyx4 commented 2 months ago

One issue in trying to fix this is that v.norm() currently returns essentially v.dot_product(v).sqrt(), so changing it to v.inner_product(v) in the IntegralLattice case would introduce some inconsistency...

DaveWitteMorris commented 2 months ago

This is not a bug, because the method is doing exactly what the documentation says it will do: the documentation of the norm method states that (by default) it returns the Euclidean norm (i.e., the ell^2-norm) of the vector.

If the name of the method is causing confusion, then perhaps it should be changed to something like p_norm, but anything like this would require discussion and a deprecation period.

yyyyx4 commented 2 months ago

Calling .norm() should compute whatever the correct meaning of "norm" is in the vector's assigned .parent() structure, IMHO.