Skip to content
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

incorporate elements of randomness to the mini notation #165

Merged
merged 8 commits into from Aug 6, 2022

Conversation

bpow
Copy link
Contributor

@bpow bpow commented Jul 29, 2022

Work in progress, not ready for merge

Currently implements the ? operator (degradeBy). Trying to implement the random choice notation (pipe-separated elements in a list), but needs randcat in order for that to work. I think I have a working peggy parser implementation, though.

@felixroos felixroos marked this pull request as draft July 29, 2022 23:15
@bpow bpow force-pushed the randomness branch 2 times, most recently from 1ab7a76 to 62f837e Compare July 31, 2022 18:20
@bpow
Copy link
Contributor Author

bpow commented Jul 31, 2022

I'm going to leave this titled as (WIP) for now, but these commits do result in implementation of the ? notation for degradeBy and | notation for random choice among cycles.

I've rebased against main for some clarity and deferred updating of krill-parser.js to its own commit rather than in each of the first 2 commits to avoid extra churn to this generated file. Happy to just squash everything together if you'd prefer.

Rather than just use randcat, I followed the logic of 5501a8 from Tidal to keep a "seed" as part of the parser state and move within rand by a fraction of that seed (incremented each time it is used). This prevents things like having 'c4? e4? g4?' having each note use the same segment from rand (so they would either all be played or all muted in a given cycle). Similarly, it keeps "[c3|e3|a3],[c4|e4|a4]" from just playing octaves each time -- instead it sometimes plays octaves, but randomly sometimes plays pairs of notes from the A minor triad, one each in octave 3 and 4.

I'm not sure exactly how to best write tests for this since there's randomness involved. Doing so might have to involve either testing that one or another of the alternatives is chosen as the first cycle, or injecting a specific rand (with specific time seed) into the parser.

@yaxu
Copy link
Member

yaxu commented Jul 31, 2022

Great! One way to test is to add an example that uses ?, then a test is automatically done to check that the values are always the same.

@bpow bpow force-pushed the randomness branch 2 times, most recently from 80d6df4 to ddf6c91 Compare August 5, 2022 21:55
@bpow bpow mentioned this pull request Aug 6, 2022
14 tasks
this will require update to krill-parser.js (aggregated
in later commit) to work properly
this will require update to krill-parser.js (aggregated
in later commit) to work properly
This would no longer match with Tidal (which uses
0.0001), but reduces the correlation among the
the different random streams in the mini-notation's
parsing of ? and |
This really just tests that the ? operator faithfully gets represented
as degradeBy(0.5)
The tests are probabilistic, so it is possible that if the
pseudo-random number generator changes in the future, we might
get results that fail. They work for the current PRNG, though,
and use boundaries for the number of values of different types
such that there should only be about a 1% probability that the
tests would fail by chance assuming that the PRNG returns
evenly distributed values.
@bpow bpow changed the title (WIP) incorporate elements of randomness to the mini notation incorporate elements of randomness to the mini notation Aug 6, 2022
@bpow
Copy link
Contributor Author

bpow commented Aug 6, 2022

I'm ready to have someone else look over this. It is all rebased to main as of now, has some tests within packages/mini and also a snapshot-based test.

There's probably some more work that could be done to the pegjs grammar (this doesn't address #176, for instance, but I think that involves enough other things to warrant a separate pull request flow).

I tried to have the commits make logical sense, but would be happy to squash things if that's the local convention.

@bpow bpow marked this pull request as ready for review August 6, 2022 22:34
@felixroos
Copy link
Collaborator

This looks really good! no objections here. We don't have any conventions when it comes to squashing commits, I think it's pretty readable as it is. Thank you so much!

@felixroos felixroos merged commit 6025dd6 into tidalcycles:main Aug 6, 2022
@felixroos
Copy link
Collaborator

felixroos commented Aug 6, 2022

I just noticed some strange behavior in the bossaRandom example:

const chords = "<Am7 Am7 Dm7 E7>"
const roots = chords.rootNotes(2)

stack(
  chords.voicings(['F4', 'A5']).struct(
  ` x@2   ~ x ~ ~ ~ x |
    x?  ~ ~ x@3   ~ x |
    x?  ~ ~ x ~ x@3`).note().gain(.5),
  roots.struct("x [~ x?0.2] x [~ x?] | x!4 | x@2 ~ ~ ~ x x x")
  .transpose("0 7").note()
).slow(2).pianoroll().s('sawtooth').out();

With this snippet, I consistently get a missing event on the downbeat of the second playthrough of the chords (when starting from a fresh load). The odd thing is that the downbeat of that first phrase has no "?" so it should play in every case. The problem seems to vanish when I remove the chords

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants