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: Reed-Solomon decoder accept FFT domain #309

Merged
merged 16 commits into from
Jun 8, 2023
Merged

Conversation

ggutoski
Copy link
Contributor

@ggutoski ggutoski commented Jun 8, 2023

Description

close: #261

  • Builds upon Refactor Reed Solomon code #256. Merge target is rs-code-fft instead of main so you can see the stuff I added to Refactor Reed Solomon code #256. If desired we could change the merge target to main and close Refactor Reed Solomon code #256.
  • New function reed_solomon_erasure_decode_rou accepts FFT domain and wraps reed_solomon_erasure_decode.
  • House cleaning: change reed_solomon_erasure_encode into an iterator adaptor. (ie. It now returns an iterator.) It's not obvious/easy to do the same for reed_solomon_erasure_decode so I didn't do that.

Warning

(UPDATE: fixed in 08e870f . See #309 (comment) . )

Naive implementation of reed_solomon_erasure_decode_rou computes each eval point in linear time. Ideally we would like constant-time random access to domain points without allocating and populating a vec for all the points. How best to achieve that?

// TODO(Gus) nth runtime is linear in index!
(domain.elements().nth(index).unwrap(), eval)


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (main)
  • Linked to GitHub issue with discussion and accepted design OR have an explanation in the PR that describes this work.
  • Wrote unit tests
  • Updated relevant documentation in the code
  • Added a relevant changelog entry to the Pending section in CHANGELOG.md
  • Re-reviewed Files changed in the GitHub PR explorer

@ggutoski ggutoski added the sprint4 Sprint 4 label Jun 8, 2023
@ggutoski ggutoski requested a review from mrain June 8, 2023 17:11
@mrain
Copy link
Contributor

mrain commented Jun 8, 2023

Generally LGTM. I think we can close the previous PR and directly merge this one to the main branch.

@ggutoski ggutoski changed the base branch from rs-code-fft to main June 8, 2023 19:46
@ggutoski ggutoski requested a review from mrain June 8, 2023 19:48
Copy link
Contributor

@mrain mrain left a comment

Choose a reason for hiding this comment

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

LGTM!

@ggutoski ggutoski merged commit 994fe10 into main Jun 8, 2023
2 checks passed
@ggutoski ggutoski deleted the rs-code-fft-gus branch June 8, 2023 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sprint4 Sprint 4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reed-Solomon decoder accept FFT domain
2 participants