Skip to content

feat(gfpoly_factor_edf): add one testcase multiple times#39

Merged
Sarsum merged 1 commit intoSarsum:mainfrom
tametsi:feat/testcases-edf
Nov 25, 2025
Merged

feat(gfpoly_factor_edf): add one testcase multiple times#39
Sarsum merged 1 commit intoSarsum:mainfrom
tametsi:feat/testcases-edf

Conversation

@tametsi
Copy link
Contributor

@tametsi tametsi commented Nov 25, 2025

Yep, that's the same EDF testcase 20 times.

This testcase failed for me if a bad random polynomials have been chosen (in approx. 30%-40% of the cases).

Yep, that's the same testcase 20 times as this failed for me if a bad
random polynomials have been choosen.
@Sarsum
Copy link
Owner

Sarsum commented Nov 25, 2025

works for me 🚀

I am also removing a value in certain cases, is that not part of the default algorithm?

@niri81
Copy link
Collaborator

niri81 commented Nov 25, 2025

I am also removing a value in certain cases, is that not part of the default algorithm?

Wdym?

@Sarsum
Copy link
Owner

Sarsum commented Nov 25, 2025

I was just curious why the issue arose, as the slides contain this edgecase:

        for u in z:
            if deg(u) > d:
                j = gcd(u, g)
                if (j != 1) and (j != u):
                    # u is factored
                    z.remove(u)
                    z.add(j)
                    z.add(u / j)

@tametsi
Copy link
Contributor Author

tametsi commented Nov 25, 2025

I am also removing a value in certain cases, is that not part of the default algorithm?

It is. But my implementation sets the value of this factor to nil which might lead to a nil pointer dereference in the next loop run.

@Sarsum
Copy link
Owner

Sarsum commented Nov 25, 2025

Ahh alright :D

@tametsi
Copy link
Contributor Author

tametsi commented Nov 25, 2025

Merge it if you think having these cases is worth it.

TBH, I don't care as I test this case in my unit test, too.

@Sarsum Sarsum merged commit feeb177 into Sarsum:main Nov 25, 2025
1 check passed
@Sarsum
Copy link
Owner

Sarsum commented Nov 25, 2025

I was just curious how these arose. But they definitely might be useful for others!

@tametsi tametsi deleted the feat/testcases-edf branch November 25, 2025 13:24
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