Round sample values in physical-to-digital conversion #419
Conversation
This file contains 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
added 3 commits
September 1, 2022 14:42
The variable e_p_signal is not defined here; self.e_p_signal is what was intended.
When converting physical to digital values, the input values are
floating-point and therefore may have rounding errors. We want to
round each value to the nearest integer before calling istype, in
order to avoid adding a bias towards zero.
This applies to adc in all four modes: expanded and not expanded,
in-place and not-in-place.
("In-place" here means both that we can overwrite the original
floating-point arrays, and that the record attributes will be updated
afterwards. In "not-in-place" mode, we are operating on copies of the
original arrays.)
When converting physical to digital values, the input values are floating-point and therefore may have rounding errors. We want to ensure that the package rounds each value to the nearest integer, in order to avoid adding a bias towards zero. This test case tests all four code paths in Record.adc, and also checks that the high-level wfdb.wrsamp function works correctly for a round trip from physical to digital to physical. Note that wrsamp currently doesn't support expanded/multi-frequency data, so that is not tested.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When converting physical (floating-point) values to digital (integer) values, we generally want to round to the nearest integer, not truncate towards zero.
(Translating floating-point data to integers is more complicated than you'd think, but we should try to avoid creating new sources of errors.)
One example:
(we would of course expect these arrays to be equal.)
This fixes issue #418.
edit: I am not good at copy and paste