search

mratsim / Arraymancer

A fast, ergonomic and portable tensor library in Nim with a deep learning focus for CPU, GPU and embedded devices via OpenMP, Cuda and OpenCL backends
https://mratsim.github.io/Arraymancer/
Apache License 2.0
1.34k stars 96 forks source link

fancy indexing: Error: 'masked_axis_select' can have side effects #441

Closed brentp closed 4 years ago

brentp commented 4 years ago

the code below causes a compiler error.

import arraymancer

proc subset(T: Tensor[float32], Q: Tensor[float32]) =
    # 60% of samples have known genotype at each site
    var sel = (Q !=. -1'f).asType(float32).sum(axis=0) >. (0.6'f32 * Q.shape[0].float32)
    sel = sel and ((T !=. -1'f).asType(float32).sum(axis=0) >. (0.6'f32 * T.shape[0].float32))
    echo sel.asType(int32).sum

    var Qr = Q[_, sel]
    var Tr = T[_, sel]

full error is:

/home/brentp/src/somalier/x.nim(9, 15) template/generic instantiation of `[]` from here
/home/brentp/.nimble/pkgs/arraymancer-#head/tensor/accessors_macros_read.nim(55, 12) template/generic instantiation of `slice_typed_dispatch` from here
/home/brentp/.nimble/pkgs/arraymancer-#head/tensor/private/p_accessors_macros_read.nim(262, 38) template/generic instantiation of `masked_axis_select` from here
/home/brentp/.nimble/pkgs/arraymancer-#head/tensor/selectors.nim(206, 6) Error: 'masked_axis_select' can have side effects
mratsim commented 4 years ago

It's fixed in #441.

The || openmp iterator is tagged sideeffect but it is only used when passing -d:openmp hence a sleeping bug. I'll add nimble test_openmp to the Linux test suite to catch that.

brentp commented 4 years ago

thank you! BTW, I get error on arraymancer.nimble even after getting this fix:

 ...   Evaluating as NimScript file failed with: 
        ...     /home/brentp/src/Arraymancer/arraymancer_23553.nims(242, 21) Warning: imported and not used: 'docs' [UnusedImport]
        ... printPkgInfo() failed.
Vindaar commented 4 years ago

There's definitely some fixing to be done because of that PR. I'll try to fix it.

edit: This is great. The generated nimscriptapi.nims file uses paramCount but explicitly does not import os if nimscript is defined. So I get a failure there already.

edit2: I forgot to build nim tools again :) Nope. does not help.

edit3: there is no new release for nimble, so I have to pull nimble manually now? :| Yup. Note: apparently koch --latest nimble is a thing!

edit4: got it.