grame-cncm / faust

Functional programming language for signal processing and sound synthesis
http://faust.grame.fr
Other
2.59k stars 325 forks source link

Fix std::remainder in JAX architecture #1001

Closed DBraun closed 9 months ago

DBraun commented 9 months ago

This fixes an incorrect implementation of std::remainder in JAX. Reference: https://en.cppreference.com/w/cpp/numeric/math/remainder

Previous outputs:

>>> remainder(5.1,3)
Array(-0.9000001, dtype=float32, weak_type=True)
>>> remainder(-5.1,3)
Array(0.9000001, dtype=float32, weak_type=True)
>>> remainder(5.1,-3)
Array(2.1, dtype=float32, weak_type=True)
>>> remainder(-5.1,-3)
Array(-2.1, dtype=float32, weak_type=True)

New correct outputs:

>>> remainder(5.1,3)
Array(-0.9000001, dtype=float32, weak_type=True)
>>> remainder(-5.1,3)
Array(0.9000001, dtype=float32, weak_type=True)
>>> remainder(5.1,-3)
Array(-0.9000001, dtype=float32, weak_type=True)
>>> remainder(-5.1,-3)
Array(0.9000001, dtype=float32, weak_type=True)

Note that if we had just used jax.numpy.remainder:

>>> jnp.remainder(5.1,3)
Array(2.1, dtype=float32, weak_type=True)
>>> jnp.remainder(-5.1,3)
Array(0.9000001, dtype=float32, weak_type=True)
>>> jnp.remainder(5.1,-3)
Array(-0.9000001, dtype=float32, weak_type=True)
>>> jnp.remainder(-5.1,-3)
Array(-2.1, dtype=float32, weak_type=True)

Update: this is okay to merge but I also need to update the architecture impulse tests file: https://github.com/grame-cncm/faust/blob/bb1567047e390f84528ae65d408b0dd627c1cd1e/tests/impulse-tests/archs/impulsejax.py#L29

sletz commented 9 months ago

Thanks, but I prefer putting this code inside the backend code generator itself, the way we do with some others (like CMajor) which also have some missing math functions. This case the missing maths functions do not have to be in each architecture file.

Done in https://github.com/grame-cncm/faust/commit/4b429eca45841272794354b3b9f6b691d7993b0e