Skip to content

Conversation

jonas-lj
Copy link
Contributor

@jonas-lj jonas-lj commented Sep 30, 2025

Implement a RS decoder with Gao's algorithm. This relies on implementations of polynomial division and extended GCD for polynomials, so the polynomial module has been extended accordingly.

@jonas-lj jonas-lj requested a review from benr-ml October 1, 2025 13:10
@jonas-lj jonas-lj changed the title Add polynomial division and xgcd Add RS decoder Oct 2, 2025
@arnab-roy
Copy link

Jonas, I ran some additional tests on the decoder. It's producing correct results and seems to be pretty efficient around n = O(100). However, it's running ~300-400s on my machine for n=1000. Is that acceptable or we want to optimize more? I can push the new tests to your branch if that helps.

@jonas-lj
Copy link
Contributor Author

jonas-lj commented Oct 4, 2025

Jonas, I ran some additional tests on the decoder. It's producing correct results and seems to be pretty efficient around n = O(100). However, it's running ~300-400s on my machine for n=1000. Is that acceptable or we want to optimize more? I can push the new tests to your branch if that helps.

That's a good point. The implementation is rather naive, so there are a few low hanging optimisations, we could try. If it breaks that much for n=1000, I suspect just using iterators could be a worthwhile optimisation. I'll work on it and get back to you.

@jonas-lj
Copy link
Contributor Author

jonas-lj commented Oct 9, 2025

Jonas, I ran some additional tests on the decoder. It's producing correct results and seems to be pretty efficient around n = O(100). However, it's running ~300-400s on my machine for n=1000. Is that acceptable or we want to optimize more? I can push the new tests to your branch if that helps.

That's a good point. The implementation is rather naive, so there are a few low hanging optimisations, we could try. If it breaks that much for n=1000, I suspect just using iterators could be a worthwhile optimisation. I'll work on it and get back to you.

@arnab-roy I found the bottleneck and got the decoding down to 120 ms for n=1024 which should be fine.

@arnab-roy
Copy link

arnab-roy commented Oct 9, 2025

@jonas-lj Amazing! I tested your new code and now the same tests are 1-2s in my machine (likely more than the numbers you got due to crude measurements and machine specs). Ran some more edge cases as well, and all working :)

@jonas-lj
Copy link
Contributor Author

jonas-lj commented Oct 9, 2025

@jonas-lj Amazing! I tested your new code and now the same tests are 1-2s in my machine (likely more than the numbers you got due to crude measurements and machine specs). Ran some more edge cases as well, and all working :)

Yea, the trick to get a 99% improvement in performance is just to start from something really bad #til

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.

3 participants