SciNim / flambeau

Nim bindings to libtorch
85 stars 3 forks source link

asTorchView doesn't work on non-declared seq #18

Closed Clonkk closed 3 years ago

Clonkk commented 3 years ago
let t = zeros([1, 2, 3].asTorchView(), kInt64)

do not compile.

The current workaround is :

let shape = [1'i64, 2, 3]
let t = zeros(shape.asTorchView(), kInt64)

We should either wrap functions taking a IntArrayRef to accept openArray in a higherl evel wrapping or modify as TorchView

HugoGranstrom commented 3 years ago

I've stumbled upon this a fair bit myself. The problem is that the expression is inline and doesn't have an address. So if we just assign it to a variable in asTorchView it does work:

template asTorchView*[T](oa: openarray[T]): ArrayRef[T] =
  let a = oa
  ArrayRef[T].init(a[0].unsafeAddr, oa.len)

Not sure if we introduce unneccecary copies though, but as oa is openarray it should only be the pointer+len that is copied into a.

HugoGranstrom commented 3 years ago

Even though this works, I still think we should expose openarray for the high-level API. asTorchView is 11 characters too long for me 😝

Clonkk commented 3 years ago

@HugoGranstrom I think this is mostly fixed and I propose we close this. I've reviewed the high level API and it uses openArray or Varargs for most procs and convert with as TorchView internally.

The low level API will still rely on asTorchView / ArrayRef so be as close as C++ Torch as possible.