I remember that JMcC limited the range of the phase when he introduced phase modulation as wave shaping. It is certainly an efficiency issue, but perhaps one modulo operation is cheap enough to justify?

Would it make sense to simply document the expected range, and direct users to modulo themselves if needed?

On 13 Apr 2013, at 11:46, Julian Rohrhuber wrote:

I remember that JMcC limited the range of the phase when he introduced phase modulation as wave shaping. It is certainly an efficiency issue, but perhaps one modulo operation is cheap enough to justify?

—
Reply to this email directly or view it on GitHub.

cannot check the code atm, but the phase wrapping is not the only problem of the current wave table oscillator implementation. it is also not completely accurate, which can cause issues when combining sinosc and fsinosc for FM or pm synthesis. I have a prototype of a more precise implementation, but it is notably slower than sinosc or osc. iirc something like 30%.
so it might be reasonable to have more than one oscillator implementation, depending on the use case.

Thanks all for the comments. I would suggest that something needs to be done to patch this problem before the next minor release, and it would either be to put a modulo in, or document the limitation. (Tim's alternative implementation: since tim suggests it as an additional ugen I'd suggest it would be good for it to go into sc3-plugins rather than having multiple implementations in core.)

So the immediate question is: do we add a modulo, or do we document the limitation and tell users to do their own modulo? Any further votes from devs?

On 17 Apr 2013, at 08:01, danstowell wrote:

So the immediate question is: do we add a modulo, or do we document the limitation and tell users to do their own modulo? Any further votes from devs?

Repeating myself a bit, but I vote for document and tell. SinOsc is used a lot in 'mass production' situations like additive synthesis, so I think it's good to keep it as cheap as possible. Those modulos will add up, and unless you've got a clever implementation there may not be any performance gain from integrating it.

I tend to agree with Scott. I'd be skeptical of slowing things down for everybody because some few might shoot themselves in the foot under some rare circumstance. (And, what's a programming language without a way to shoot yourself in the foot?)

Or at least benchmark the CPU consumption with and without a modulo embedded in SinOsc's c++ code. If that proves a negligible effect on CPU usage with a few thousand oscillators running, then I might be persuaded to change my mind. IMO the onus for that testing should be on those who want the change.

It's also consistent with the general policy for 'out of range' arguments in UGens. There have been requests for similar safety checks in other UGens, and we've rejected them for similar reasons.