-
Notifications
You must be signed in to change notification settings - Fork 327
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
Lowpass Filter Improved Uniform Sampling Check #3978
base: main
Are you sure you want to change the base?
Conversation
30f77f8
to
c8c7bab
Compare
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.
Nice work @alexbeattie42!
My main suggestion is to expose isUniform
by adding it to CommonUtilities
, with some documentation about how the uniformity checks work. You could then reference that in the updated documentation for TableUtilities::filterLowpass
.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @alexbeattie42)
OpenSim/Common/TableUtilities.cpp
line 126 at r1 (raw file):
template <typename T> std::pair<bool,T> isUniform(const std::vector<T>& x) {
I think this would be a nice addition to CommonUtilities, as it might be useful for other table manipulation tools in OpenSim.
OpenSim/Common/TableUtilities.cpp
line 189 at r1 (raw file):
// Resample if the sampling interval is not uniform. if (!uniformlySampled) {
Could you add some documentation in the header file describing the conditions under which a table will be resampled?
c705176
to
9d0880a
Compare
85312a2
to
b8a25c1
Compare
0998a15
to
308f79c
Compare
308f79c
to
b4ba7ca
Compare
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.
@nickbianco could you take another look?
Reviewable status: 0 of 6 files reviewed, 2 unresolved discussions (waiting on @nickbianco)
OpenSim/Common/TableUtilities.cpp
line 126 at r1 (raw file):
Previously, nickbianco (Nick Bianco) wrote…
I think this would be a nice addition to CommonUtilities, as it might be useful for other table manipulation tools in OpenSim.
Done.
OpenSim/Common/TableUtilities.cpp
line 189 at r1 (raw file):
Previously, nickbianco (Nick Bianco) wrote…
Could you add some documentation in the header file describing the conditions under which a table will be resampled?
Done.
Fixes issue #3968
Brief summary of changes
Implement an
isUniform
method to check if the vector is uniformly sampled. The implementation loosely follows the idea of the Matlab isUniform function. The implementation calculates the tolerance in this way:This may also fix this issue but I have not tested that myself yet.
Testing I've completed
Tested locally with this example and it fixes this issue. Will add unit tests soon.
Looking for feedback on...
@nickbianco and @aymanhab
Signal
?CHANGELOG.md (choose one)
This change is