Resync the resampler on system time changes (take 2) #999
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Matthias Wende <[email protected]>
Adds a boolean parameter if an extra period should be added to the calculated window end or not. Signed-off-by: Matthias Wende <[email protected]>
Signed-off-by: Matthias Wende <[email protected]>
Signed-off-by: Matthias Wende <[email protected]>
Signed-off-by: Matthias Wende <[email protected]> Co-authored-by: Leandro Lucarella <[email protected]>
Signed-off-by: Matthias Wende <[email protected]> Co-authored-by: Leandro Lucarella <[email protected]>
Tests should try to be as close as possible as real code. Signed-off-by: Leandro Lucarella <[email protected]>
Future changes require adding more code to this module, which is already quite big. This commit makes it a package and split the module into a few sub-modules to make it more manageable. Signed-off-by: Leandro Lucarella <[email protected]>
`ResamplingFunction` is used by `ResamplerConfig`, which is public, and `SourceProperties` is used by `ResamplingFunction`, so both should be public too. Signed-off-by: Leandro Lucarella <[email protected]>
Using a type alias forces us to use strings to specify types because Python can't use forward declarations for statements. Also using a protocol allows for more control, as even when we don't use it for now, it will also check function argument names if `/` is not used. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
`Sample` can have `None` as value, but the resampler buffer (and function) never get any `None` value stored/passed. The reason is saving a sample with no value provides no extra information and would make the resampling function implementation more complicated, as it would need to check values for `None`. Currently the resampling function will never receive a `None` value but it still needs to check for them for type-checking reasons. This commit uses a `tuple[datetime, Quantity]` to store samples in the buffer and pass it to the resampling function. This way it is trivially clear that values can't be `None` in this context. Signed-off-by: Leandro Lucarella <[email protected]>
The `average` function is just a mean function and Python already offers a function to calculate a mean, so we use that instead. We also wrap `fmean` to match the `ResamplingFunction` signature inline so it is clear in the documentation what the default resampling function does. Signed-off-by: Leandro Lucarella <[email protected]>
This reduces the benchmark time by about 4x. Also use `Quantity.zero()` as a shortcut. Signed-off-by: Leandro Lucarella <[email protected]>
github-actions
bot
added
part:docs
|
Replaced by: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR builds on top of #802 by using a new timer that is attached to the wall clock instead of the monotonic clock, and can be used to trigger resampling windows. It is a replacement for the channels Timer class.
This should fix the issue with the resampling windows being too small when the system clock is not in sync with the wall clock.
Status: Some code adjustments might still be needed, but for sure the big chunk of work pending are the tests, as testing this it is very complex as one need to emulate both the wall clock and the monotonic clock and the different ways they can be out of sync.