Skip to content

%lagoon argmax/argmin jets don't work right #1001

@yapishu

Description

@yapishu

the argmax and argmin jets from lagoon return len - i - 1 instead of i; for an N-element ray whose max sits at logical index k, the jet returns N-1-k. verified by comparing (argmax:la logits) against a numpy reference (i found this bug trying to reproduce gpt2 inference). proposed fix in urbit/numerics#22

in lagoon/vere/noun/jets/i/lagoon.c:

case 5: {
  float32_t max_val32 = ((float32_t*)x_bytes)[0];
  for (c3_d i = 0; i < len_x; i++) {
     if(f32_gt(((float32_t*)x_bytes)[i], max_val32)) {
       max_val32 = ((float32_t*)x_bytes)[i];
       max_idx = (len_x - i - 1);   // <-- bug
     }
  }
  break;}

u3r_bytes(0, syz_x, x_bytes, x_data) pulls the atom bytes lsb-first, and atoms store ray element 0 at lsb — so ((float32_t*)x_bytes)[i] for i=0..len-1 iterates logical elements 0..len-1 in order. every other jet in the same file (add/sub/mul/mmul/…) reads x_bytes[i] without any index reversal, only argmax/argmin introduce the (len_x - i - 1) issue

this wasn't picked up by tests because lagoon/desk/tests/lib/ has no tests for argmax or argmin. the hoon reference is index-correct, only the jet flips the index

urbit -F zod
> |new-desk %lagoon
> |mount %lagoon
git clone https://github.com/urbit/numerics
cp -Lr numerics/lagoon/desk/* zod/lagoon/
echo '[%zuse 409]' > zod/lagoon/sys.kelvin

zod/lagoon/gen/test-argmax.hoon:

/-  ls=lagoon
/+  *lagoon
:-  %say
|=  *
:-  %noun
=/  la  (lake %n)
=/  meta=meta:ls  [~[5] 5 %i754 ~]
::  build [1.0 4.0 2.0 0.5 3.0] — max is at logical index 1
=/  r  (zeros:la meta)
=/  r  (set-item:la r ~[0] .1)
=/  r  (set-item:la r ~[1] .4)
=/  r  (set-item:la r ~[2] .2)
=/  r  (set-item:la r ~[3] .0.5)
=/  r  (set-item:la r ~[4] .3)
=/  idx  (argmax:la r)
~&  >  ['argmax of [1 4 2 0.5 3]' idx]
~&  >  ?:  =(idx 1)  'PASS — index 1'
       ?:  =(idx 3)  'FAIL — reversed to (len-1-1)=3'
       'FAIL — other'
~
> |commit %lagoon
> +lagoon!test-argmax

output:

>   ['argmax of [1 4 2 0.5 3]' 3]
>   'FAIL — reversed to (len-1-1)=3'

fix:

    max_idx = i;
    // or, for argmin:
    min_idx = i;

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions