-
Notifications
You must be signed in to change notification settings - Fork 12
Added: Modulo, functional convenience, LFImpulse AGen #91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: feature-agen
Are you sure you want to change the base?
Conversation
…ed rounding operations for apply-shortform
|
There seem to be some git issues with the AudioIn and Klang AGens, which are not part of this Pull Request, but appear to be so... |
|
Looks good to me |
thomas-hermann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contributions. There are minor issues:
- LFImpulse should come with unit tests
- I recommend adding LFImpulse to examples/pya-examples-agen-tour.ipynb
- We should discuss the issue of negative frequencies, and furthermore the need for Nyquist checks in LF* AGens (Ideally we'd be consistent in AGens).
- SC3 parity should be reflected first (not necessarily a critical issue - we might see the need to decide differently, but with arguments. This is something we should look at and document in a pya-development gitlab repo.
| def _generate_single(self, sample_count: int, start: int = 0) -> np.ndarray: | ||
| # To ensure first output is 1 if input phase == 0 | ||
| m_phase = self.state.data.get("m_phase", -sys.float_info.min) | ||
| m_input_phase = self.state.data.get("m_input_phase", self.nodes["phase"][0]) # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intended behaviour requires an initial positive values of freq! Let's discuss whether negative freq could make sense (i.e. phase decrements, going backwards...). If yes, multiplying the initial value with signbit of initial freq would solve the issue. However, that requires to self.nodes["freq"] before initializing m_phase.
| from enum import Enum | ||
| from typing import TYPE_CHECKING, Iterable, Sequence | ||
| import threading | ||
| import sys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what requires import of sys here?
| # Use a cumsum here to account for varying frequencies | ||
| phases = np.cumsum( | ||
| np.concatenate( | ||
| ([m_phase], np.minimum(self.nodes["freq"], self.sr/2) / self.sr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as this is an LF (aka non-bandlimited) version of Impulse, I wonder whether we should forego checks for Nyquist. (which would also make computation slightly faster.
Related to the comment above (line 272): this check does not prevent freqs below -self.sr/2.
Modulo
Added the ability to use the modulo operator
GenOrNum % GenOrNum -> AGen.Functional Convenience
Added functional style functions
.mod,.sub,.div,.powas an alternative form of applying the operator.Added functional style functions
.floor,.ceil,.roundas a short form of.apply(...).Notes: As a generic implementation for all numpy-functions as an
.apply(...)-shortcut, a general cleanup is necessary. Especially the last three ones should be removed along many others. In the meantime they could be handy.LFImpulse AGen
Added an non band limited Impulse AGen.
The general behavior e.g. when modulating the phase is the same as the SuperCollider Impulse, however a sample precise parity check is yet to be done.