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

Invalid_argument("index out of bounds") with Algodiff #623

Open
talex5 opened this issue Aug 15, 2022 · 3 comments
Open

Invalid_argument("index out of bounds") with Algodiff #623

talex5 opened this issue Aug 15, 2022 · 3 comments

Comments

@talex5
Copy link

talex5 commented Aug 15, 2022

This program adds two arrays together using Algodiff:

open Owl

module AD = Owl.Algodiff.D

let a = AD.Arr (Arr.zeros [| 1; 1 |])
let b = AD.make_reverse (AD.Arr (Arr.zeros [| 1 |])) (AD.tag ())

let () =
  let x = AD.Maths.(a + b) in
  AD.Mat.print x;
  AD.reverse_prop (F 1.) x;
  AD.Mat.print (AD.adjval b)

It fails at the reverse_prop step with:

   C0 
R0  0 
Fatal error: exception Invalid_argument("index out of bounds")
Raised by primitive operation at Owl_utils_array.sort_fill in file "src/base/misc/owl_utils_array.ml", line 483, characters 17-22
Called from Owl_utils.squeeze_continuous_dims in file "src/base/misc/owl_utils.ml", line 166, characters 13-67
Called from Owl_dense_ndarray_generic.sum_reduce in file "src/owl/dense/owl_dense_ndarray_generic.ml", line 10117, characters 16-59
Called from Owl_algodiff_ops.Make.Maths._sum_reduce.(fun).ff_arr in file "src/base/algodiff/owl_algodiff_ops.ml", line 584, characters 36-54
Called from Owl_algodiff_ops_builder.Make.build_piso.(fun).f.r_c_d.adjoint in file "src/base/algodiff/owl_algodiff_ops_builder.ml", line 364, characters 37-66
Called from Owl_algodiff_reverse.Make.reverse_push.push in file "src/base/algodiff/owl_algodiff_reverse.ml", line 42, characters 20-37
Called from Owl_algodiff_reverse.Make.reverse_prop in file "src/base/algodiff/owl_algodiff_reverse.ml" (inlined), line 53, characters 4-20
Called from Dune__exe__Test in file "test.ml", line 11, characters 2-26

I assume that since the forwards part works, the reverse step should work too. I guess it's something to do with the shape of b. If I use [|1; 1|] then it works.

@tachukao
Copy link
Member

I'm not entirely sure if this is the issue, but I think it might have to do with F 1.not being the same shape as x. I would try using an array of ones instead of F 1..

@talex5
Copy link
Author

talex5 commented Aug 17, 2022

I would try using an array of ones instead of F 1.

That doesn't help:

utop # #require "owl-top";;
utop # open Owl

module AD = Owl.Algodiff.D

let a = AD.Arr (Arr.zeros [| 1; 1 |])
let b = AD.make_reverse (AD.Arr (Arr.zeros [| 1 |])) (AD.tag ())

let () =
  let x = AD.Maths.(a + b) in
  AD.Mat.print x;
  AD.reverse_prop (Arr (Arr.ones [| 1 |])) x;
  AD.Mat.print (AD.adjval b);;

   C0 
R0  0 
Exception: Invalid_argument "index out of bounds".

Anyway, I can fix this myself by extending b. I'm just reporting it in case you want to fix it.

@mseri
Copy link
Member

mseri commented Aug 17, 2022

By mistake, I just found out that it does not fail the second time... Try this

try AD.reverse_prop (Arr (Arr.ones [| 1 |])) x with e -> AD.reverse_prop (Arr (Arr.ones [| 1 |])) x;

or

try AD.reverse_prop (F 1.) x with e -> AD.reverse_prop (F 1.) x;

and it will return unit fine 😮

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

No branches or pull requests

3 participants