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

Propose Architecture and Probe <-> Server API #11

Merged
merged 2 commits into from
Apr 20, 2023
Merged

Conversation

goodhoko
Copy link
Member

I put this into the README for no other place seemed better and so that we can have a PR over this. It's written in a definitive style for brevity but, please, read it as a proposal.

I'll raise some questions/caveats below.

}
```

The frequency of events encodes how fast a cog spins and the quality encodes the power transmitted through it. (We can add more dimensions later.) We also assume that
Copy link
Member Author

@goodhoko goodhoko Apr 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of relying on the event frequency to be transmitted over the wire we may instead make the probes to send messages regularly (Eg. every 0.1s) and encode the actual event frequency as an attribute of the regularly sent message.

I.e. a message then would be something like

Message {
    event_type: EventType,
    // number of occurences of the events in the last 0.1 s
    occurences: u32,
    // the sum of qualities
    quality: f32,
}

This could

  1. save bandwith
  2. curcumvent timing/ordering/buffering issues in the network layer

OTOH we'd also loose some resolution that could be used for sound synthesis.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of relying on the event frequency to be transmitted over the wire we may instead make the probes to send messages regularly (Eg. every 0.1s) and encode the actual event frequency as an attribute of the regularly sent message.

Another alternative would be to let probes (aggregators) encode the frequency itself (synthesis server would assume it stays the same until update?). That has an advantage over some assumed fixed interval that probes could send update as fast or as slow as the value actually changes.

The disadvantage is that we could keep playing for probes that die.

Also, is the interface (Rust type) the same before aggregation and after it? I.e. do we have some Event and AggregatedEvent, or does Event by itself (at least some types) need to support aggregation internally? I can imagine both variants.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like starting with the simplest (as in the original proposal) and evolve over time. We can revisit the idea of occurrences and frequency later.

A thought for later: I think I'm starting to prefer making probes as dumb as possible and let the server handle most of the complexities.

  • The server knows more about kinds of sound sources it can play and other events from concurrent probes, which may influence how to interpret a particular series of events. Disk reads could be thought of as a changing frequency value, or a series of discrete events.
  • We can record incoming probe events and play back on the server side to quickly iterate on aggregation and synthesis.
  • We probably want to encode timestamp in the event struct as well, so we are not influenced by the network conditions. It allows us to send Vec<Event> to reduce serialization and network overhead.

README.md Show resolved Hide resolved
@goodhoko goodhoko mentioned this pull request Apr 19, 2023
Copy link
Member

@strohel strohel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've responded to questions with more questions, hehe. Consider this as a starting point, we could finish sync if that's more efficient.

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
}
```

The frequency of events encodes how fast a cog spins and the quality encodes the power transmitted through it. (We can add more dimensions later.) We also assume that
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of relying on the event frequency to be transmitted over the wire we may instead make the probes to send messages regularly (Eg. every 0.1s) and encode the actual event frequency as an attribute of the regularly sent message.

Another alternative would be to let probes (aggregators) encode the frequency itself (synthesis server would assume it stays the same until update?). That has an advantage over some assumed fixed interval that probes could send update as fast or as slow as the value actually changes.

The disadvantage is that we could keep playing for probes that die.

Also, is the interface (Rust type) the same before aggregation and after it? I.e. do we have some Event and AggregatedEvent, or does Event by itself (at least some types) need to support aggregation internally? I can imagine both variants.

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
Co-authored-by: Matěj Laitl <[email protected]>
@skywhale skywhale requested a review from PabloMansanet April 19, 2023 13:55
@skywhale
Copy link
Member

@PabloMansanet Do you want to take a quick look? We might want to move on and start experimenting / implementation.

Copy link
Member

@skywhale skywhale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we were able to have enough discussion on this to move on. Experimetation will inform us more.

@PabloMansanet
Copy link
Contributor

Sorry, I totally missed this because I'm not getting slack notifications 😅 I thought we had registered this repository with the bot!

Please go ahead with experimentation. I'm about to leave now but I'll have more thoughts tomorrow. Looking good as a first approach though!

Copy link
Member

@strohel strohel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has served as a good opportunity for discussion. But perhaps we can merge this soon to get more up-to-date README? We can still continue some specific dicsussions here and elsewhere.

@goodhoko goodhoko merged commit bbce55b into main Apr 20, 2023
@goodhoko goodhoko deleted the propose-architecture branch April 20, 2023 11:03
@strohel strohel mentioned this pull request Apr 22, 2023
@goodhoko goodhoko mentioned this pull request Apr 26, 2023
2 tasks
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.

4 participants