Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Regression (related to static types?) #5199

Closed
andreaferretti opened this issue Jan 9, 2017 · 8 comments
Closed

Regression (related to static types?) #5199

andreaferretti opened this issue Jan 9, 2017 · 8 comments
Assignees

Comments

@andreaferretti
Copy link
Collaborator

I have just updated nim to the new 0.16 and I am checking that all my libraries still work.

While compiling linalg (just clone the repo and then nimble test) I get the error Error: ordinal type expected here.

I think this may a a compiler regression in handling static types since it only appears in functions with static types parameters (if I make that function compile by inlining the template, more errors of this kind arise)

@andreaferretti
Copy link
Collaborator Author

git bisect found bf612a7 responsible for the regression

@andreaferretti
Copy link
Collaborator Author

Seems fixed in 28005cf

@andreaferretti
Copy link
Collaborator Author

The same problem seems to be appearing again in the issue linked above.

I am reopening this

@zah zah self-assigned this May 13, 2017
@zah
Copy link
Member

zah commented Jun 20, 2017

@andreaferretti, is this still relevant? I'll be working on the remaining Static[T] issues this week and I'd like to look at this, but I'm not sure what are the reproduction steps here.

@zah zah added the Static[T] label Jun 20, 2017
@andreaferretti
Copy link
Collaborator Author

andreaferretti commented Jun 20, 2017

It does not affect me directly, mainly because I have forked linalg into a new library - neo - that does not make use of static types.

Still,, as of latest commit, I see that the error linked above still fails.

Step to reproduce: install linalg, then try to compile

import linalg

let data = matrix([
                [1.2, 0.7],
                [-0.3, -0.5],
                [3.0, 0.1],
                [-0.1, -1.0],
                [-1.0, 1.1],
                [2.1, -3]
                ])
echo data

Bringing in @mratsim that may be interested as well. Also, it may be related to #5645.

By the way, the way matrix is defined is

type
  Matrix32*[M, N: static[int]] = object
    order: OrderType
    data: ref array[N * M, float32]
  Matrix64*[M, N: static[int]] = object
    order: OrderType
    data: ref array[M * N, float64]

type DoubleArray32[M, N: static[int]] = array[M, array[N, float32]]
type DoubleArray64[M, N: static[int]] = array[M, array[N, float64]]

proc matrix*[M, N: static[int]](xs: DoubleArray32[M, N], order: OrderType = colMajor): Matrix32[M, N] =
  makeMatrix(M, N, proc(i, j: int): float32 = xs[i][j], order)

proc matrix*[M, N: static[int]](xs: DoubleArray64[M, N], order: OrderType = colMajor): Matrix64[M, N] =
  makeMatrix(M, N, proc(i, j: int): float64 = xs[i][j], order)

where makeMatrix is a certain complicated function which I think is not relevant (anyway it's here)

template isStatic(a: typed): bool =
  compiles(proc() =
    const v = a)

template makeMatrixPrivate(M, N, f, order, result: untyped) =
  result.order = order
  if order == colMajor:
    for i in 0 .. < M:
      for j in 0 .. < N:
        result.data[j * M + i] = f(i, j)
  else:
    for i in 0 .. < M:
      for j in 0 .. < N:
        result.data[i * N + j] = f(i, j)

template makeSMatrixPrivate(M, N, f, order, result: untyped) =
  new result.data
  result.order = order
  makeMatrixPrivate(M, N, f, order, result)

template makeDMatrixPrivate(M, N, f, order, result: untyped, A: typedesc) =
  new result
  result.data = newSeq[A](M * N)
  result.M = M
  result.N = N
  makeMatrixPrivate(M, N, f, order, result)

proc makeSMatrix(M, N: static[int], f: proc (i, j: int): float32, order: OrderType): Matrix32[M, N] =
  makeSMatrixPrivate(M, N, f, order, result)

proc makeDMatrix(M, N: int, f: proc (i, j: int): float32, order: OrderType): DMatrix32 =
  makeDMatrixPrivate(M, N, f, order, result, float32)

proc makeMatrix*(M: int or static[int], N: int or static[int], f: proc (i, j: int): float32, order: OrderType = colMajor): auto =
  when M.isStatic and N.isStatic: makeSMatrix(M, N, f, order)
  else: makeDMatrix(M, N, f, order)

proc makeSMatrix(M, N: static[int], f: proc (i, j: int): float64, order: OrderType): Matrix64[M, N] =
  makeSMatrixPrivate(M, N, f, order, result)

proc makeDMatrix(M, N: int, f: proc (i, j: int): float64, order: OrderType): DMatrix64 =
  makeDMatrixPrivate(M, N, f, order, result, float64)

proc makeMatrix*(M: int or static[int], N: int or static[int], f: proc (i, j: int): float64, order: OrderType = colMajor): auto =
  when M.isStatic and N.isStatic: makeSMatrix(M, N, f, order)
  else: makeDMatrix(M, N, f, order)

@Araq
Copy link
Member

Araq commented Jul 1, 2018

Please create an isolated test case for this bug.

@andreaferretti
Copy link
Collaborator Author

Here it is - sorry if it is pretty convoluted, but I could not simplify it further:

type
  OrderType = enum
    colMajor = 123, rowMajor = 124
  Vector32*[N: static[int]] = ref array[N, float32]
  Vector64*[N: static[int]] = ref array[N, float64]
  Matrix32*[M, N: static[int]] = object
    order: OrderType
    data: ref array[N * M, float32]
  Matrix64*[M, N: static[int]] = object
    order: OrderType
    data: ref array[M * N, float64]

type DoubleArray32[M, N: static[int]] = array[M, array[N, float32]]
type DoubleArray64[M, N: static[int]] = array[M, array[N, float64]]

template isStatic(a: typed): bool =
  compiles(proc() =
    const v = a)

template makeMatrixPrivate(M, N, f, order, result: untyped) =
  result.order = order
  if order == colMajor:
    for i in 0 ..< M:
      for j in 0 ..< N:
        result.data[j * M + i] = f(i, j)
  else:
    for i in 0 ..< M:
      for j in 0 ..< N:
        result.data[i * N + j] = f(i, j)

template makeSMatrixPrivate(M, N, f, order, result: untyped) =
  new result.data
  result.order = order
  makeMatrixPrivate(M, N, f, order, result)

proc makeSMatrix(M, N: static[int], f: proc (i, j: int): float64, order: OrderType): Matrix64[M, N] =
  makeSMatrixPrivate(M, N, f, order, result)

proc makeMatrix*(M: int or static[int], N: int or static[int], f: proc (i, j: int): float64, order: OrderType = colMajor): auto =
  when M.isStatic and N.isStatic: makeSMatrix(M, N, f, order)
  else: makeDMatrix(M, N, f, order)

proc matrix*[M, N: static[int]](xs: DoubleArray32[M, N], order: OrderType = colMajor): Matrix32[M, N] =
  makeMatrix(M, N, proc(i, j: int): float32 = xs[i][j], order)

proc matrix*[M, N: static[int]](xs: DoubleArray64[M, N], order: OrderType = colMajor): Matrix64[M, N] =
  makeMatrix(M, N, proc(i, j: int): float64 = xs[i][j], order)

template atPrivate(M, N, m, i, j: untyped, A: typedesc): auto =
  if m.order == colMajor:
    let data = cast[ref array[N, array[M, A]]](m.data)
    data[j][i]
  else:
    let data = cast[ref array[M, array[N, A]]](m.data)
    data[i][j]

template atPrivateD(m, i, j: untyped): auto =
  if m.order == colMajor: m.data[j * m.M + i]
  else: m.data[i * m.N + j]

proc at*[M, N: static[int]](m: Matrix64[M, N], i, j: int): float64 {. inline .} = atPrivate(M, N, m, i, j, float64)

proc row*[M, N: static[int]](m: Matrix64[M, N], i: int): Vector64[N] {. inline .} =
  new result
  for j in 0 ..< N:
    result[j] = m.at(i, j)

proc `$`*[N: static[int]](v: Vector32[N] or Vector64[N]): string =
  result = "[ "
  for i in 0 ..< N - 1:
    result &= $(v[i]) & "\n  "
  result &= $(v[N - 1]) & " ]"

proc toStringHorizontal[N: static[int]](v: Vector32[N] or Vector64[N]): string =
  result = "[ "
  for i in 0 ..< N - 1:
    result &= $(v[i]) & "\t"
  result &= $(v[N - 1]) & " ]"

proc `$`*[M, N: static[int]](m: Matrix64[M, N]): string =
  result = "[ "
  for i in 0 ..< M - 1:
    result &= toStringHorizontal(m.row(i)) & "\n  "
  result &= toStringHorizontal(m.row(M - 1)) & " ]"

let data = matrix([
                [1.2, 0.7],
                [-0.3, -0.5],
                [3.0, 0.1],
                [-0.1, -1.0],
                [-1.0, 1.1],
                [2.1, -3]
                ])
echo data

@andreaferretti
Copy link
Collaborator Author

Seems fixed with latest devel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants