Sounds: long-established demo no longer working properly

[martin-henz]

Expected Behavior

Bohemian rapsody cover should be heard when playing
https://share.sourceacademy.org/g9n0m
using Source §2 or Source §2 native

Current Behavior

Something else is heard that has some of the harmonies of Bohemian rapsody, but sounds very different.

Failure Information (for bugs)

Steps to Reproduce

Run
https://share.sourceacademy.org/g9n0m

Context

MacBook Pro, x86, MacOS Monterey

[RichDom2185]

I can reproduce this. It seems like consecutively is playing the list in reverse order for some reason. Changing The following line as follows:

Results in the sound playing properly.

Will need to investigate.

[tsinyee]

i think the issue might be in js-slang repo under src/stdlib/list.ts accumulate function, related to the changes made in PR source-academy/js-slang#1388

the current implementation of accumulate reverses the result and modules is making use of the stdlib/list in js-slang

export function accumulate<T, U>(acc: (each: T, result: U) => any, init: U, xs: List): U {
  const recurser = (xs: List, result: U): U => {
    if (is_null(xs)) return result

    return recurser(tail(xs), acc(head(xs), result))
  }

  return recurser(xs, init)
}

[leeyi45]

Hmm this one's on me. I'll fix this and incorporate some tests with my latest js-slang PR

[martin-henz]

OK. Note that the accumulate function in Source Academy uses continuation passing style, to make sure it doesn't lead to JS-runtime-stack overflow:

function accumulate(f, initial, xs) {
  return $accumulate(f, initial, xs, x => x);
}
function $accumulate(f, initial, xs, cont) {
  return is_null(xs) ? cont(initial) : $accumulate(f, initial, tail(xs), x => cont(f(head(xs), x)));
}

A note on the list library: We have two versions:
(1) https://github.com/source-academy/js-slang/blob/master/src/stdlib/list.ts
That's the version available for import in modules. It is the version that has the bug. Is this version used elsewhere?
(2) https://github.com/source-academy/js-slang/blob/master/src/stdlib/list.prelude.ts
This is the version that is used in Source Academy to implement the list functions. This is done by 'eval'ing the string that has the source code for the list functions. One reason for this is that the display values of the functions are nice clean JavaScript without type declarations.

I think having two versions is acceptable because they seem to serve different purposes. Maybe the version (1) isn't covered by regression testing, which means the bug slipped through.

[BTW, there is a third version:
(3) https://github.com/source-academy/js-slang/blob/master/docs/lib/list.js
This version is a variant of the prelude version above and is used to generate the documentation of the list library.]

[leeyi45]

Would the following implementation work?

export function accumulate<T, U>(op: (each: T, result: U) => any, initial: U, sequence: List): U {
  // Use CPS to prevent stack overflow
  function $accumulate(f: typeof op, xs: typeof sequence, cont: (each: U) => U): U {
    return is_null(xs) ? cont(initial) : $accumulate(f, tail(xs), x => cont(f(head(xs), x)));
  }
  return $accumulate(op, sequence, x => x);
}

With this implementation the sounds demo works as expected

[martin-henz]

yes, that's correct. You can include this in a PR.

[RichDom2185]

Prof @martin-henz, we need to bump js-slang to fix this fully. Otherwise, this issue is closed via source-academy/js-slang#1463.