oscar-system / Oscar.jl

A comprehensive open source computer algebra system for computations in algebra, geometry, and number theory.
https://www.oscar-system.org
Other
343 stars 126 forks source link

Add `is_unimodular(integer matrix)` #2249

Open lkastner opened 1 year ago

lkastner commented 1 year ago

Is your feature request related to a problem? Please describe. We would like to have a method for testing integer matrices for unimodularity, i.e. determine whether the rows of an integer matrix can be completed to a lattice basis.

Describe the solution you'd like Something similar to the following would work. I am unsure though whether the first line should give an error instead.

function _is_unimodular(M::ZZMatrix)
  nrows(M) <= ncols(M) || return false
  n = nrows(M)
  return abs(det(snf(M)[:,1:n])) == 1
end

Probably the solution should happen in Hecke.

Describe alternatives you've considered I tried using is_unimodular from Hecke with some Zlattice construction but on slack we agreed that this does something different.

Additional context This is used for smoothness tests in toric geometry.

cc @HereAround @thofma

HereAround commented 1 year ago

@lkastner has added a first version of this in https://github.com/oscar-system/Oscar.jl/pull/2228. (Thank you!) If this needs to be moved/improved, people can find this first version in /src/PolyhedralGeometry/PolyhedralFan/properties.jl.

fingolfin commented 1 year ago

The definition for "unimodular matrix" I am familiar with is for square matrices only. So perhaps to get started on this, could you tell us what definition for non-square matrices you have in mind (ideally with a ref to a textbook definition, so it can be included in the docstring) ?

In principle I see no harm in having this, as long as we are very clear about what it means.

JohnAAbbott commented 1 month ago

@JohnAAbbott has some code for determining if a square integer matrix (ZZMatrix) is unimodular. The code is almost ready to become a PR. Should the function be called is_unimodular? Currently its internal working name is different.