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

feat: implement resampling within xSampling to handle array lengths larger than the original size #200

Merged
merged 9 commits into from
Nov 26, 2023

Conversation

josoriom
Copy link
Member

No description provided.

Copy link

codecov bot commented Nov 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6c2a8bb) 91.69% compared to head (48f89bc) 91.70%.

❗ Current head 48f89bc differs from pull request most recent head 32a5c04. Consider uploading reports for the commit 32a5c04 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #200      +/-   ##
==========================================
+ Coverage   91.69%   91.70%   +0.01%     
==========================================
  Files         168      168              
  Lines        3286     3305      +19     
  Branches      824      824              
==========================================
+ Hits         3013     3031      +18     
- Misses        266      267       +1     
  Partials        7        7              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@josoriom
Copy link
Member Author

@lpatiny I found a function (xSampling) in this package that performs down-sampling

@josoriom josoriom changed the title feat: add xResampling function feat: implement resampling within xSampling to handle array lengths larger than the original size Nov 24, 2023
@josoriom josoriom marked this pull request as ready for review November 24, 2023 19:29
Copy link
Member

@lpatiny lpatiny left a comment

Choose a reason for hiding this comment

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

I would like you add the following test case (and I'm not convinced about the current result). Not sure what you expect the algorithm to do.

xSampling([0], { length: 2 });
xSampling([1, 2], { length: 3 });
xSampling([1, 2], { length: 5 });
xSampling([1, 2, 3], { length: 5 });
xSampling([1, 2, 4], { length: 5 });
xSampling([1, 2, 4], { length: 7 });
xSampling([1, 2, 3], { length: 4 });
xSampling([1, 2, 4], { length: 4 });
xSampling([1, 2, 1], { length: 4 });
xSampling([1, 2, -1], { length: 5 });

Do you have a reference link to the current implementation because the results seems difficult to justify.

@josoriom
Copy link
Member Author

I would like you add the following test case (and I'm not convinced about the current result). Not sure what you expect the algorithm to do.

xSampling([0], { length: 2 });
xSampling([1, 2], { length: 3 });
xSampling([1, 2], { length: 5 });
xSampling([1, 2, 3], { length: 5 });
xSampling([1, 2, 4], { length: 5 });
xSampling([1, 2, 4], { length: 7 });
xSampling([1, 2, 3], { length: 4 });
xSampling([1, 2, 4], { length: 4 });
xSampling([1, 2, 1], { length: 4 });
xSampling([1, 2, -1], { length: 5 });

Do you have a reference link to the current implementation because the results seems difficult to justify.

Defining the step size was where I made the mistake.
The source from wikipedia Linear interpolation

$$ y = y_0 \cdot \left(1 - \frac{x - x_0}{x_1 - x_0}\right) + y_1 \cdot \frac{x - x_0}{x_1 - x_0} $$

result[i] = array[floor] * (1 - diff) + array[ceil] * diff;

where

let diff = currentIndex - floor;

and

ceil - floor = 1

@lpatiny lpatiny merged commit fdeca70 into main Nov 26, 2023
1 check passed
@lpatiny lpatiny deleted the xResampling branch November 26, 2023 07:06
@aiday-mar aiday-mar removed their request for review November 27, 2023 08:07
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.

2 participants