feat(gfpoly_powmod): add more edgecases for 0^0

[niri81]

Adds edge cases for zero polynomials. Especially the edge case for 0^0=1 is of interest (with 0, 1 as zero/one polynomial).

[Sarsum]

did not test them, but looks good according to my Base64 BE reading skills :D
lgtm 🚀

[melo-afk]

did test them and they seem to be ok 🚀

[tametsi]

Hmmmmmm... I think properly seeding your CSPRNG is important for ID generation.

Case lkashjklhakjhajahjahj (0^0 = 1) fails for me, which leads me to suspect that this case is not checked. I would hence propose to remove this case.

[Sarsum]

The testcase did fail for me as well, but thinking about it, the expectedResult provided by @niri81 should be correct.
I found the change which did the trick here while remainder.degree() >= mod.degree() --> while remainder.degree() > mod.degree() - but that is not entirely correct.
In case the greatest coefficient of your remainder is larger than the greatest coefficient of the modulus, you need one last reduction.

Edit: As I implemented Ord, PartialOrd etc. to sort the polynomials, I switched to remainder.cmp(mod).is_ge() which should check as described above. All of the testcases in this Repo are working with this change - I am a little bit worried about getting regression on kartfire though :D

[niri81]

I also had a TIL moment when checking that against Sage and fell in a bit of a rabbit hole. Apparently, this characteristic (0^0=1) is needed for very fundamental mathematical concepts.

[niri81]

Case lkashjklhakjhajahjahj (0^0 = 1) fails for me, which leads me to suspect that this case is not checked. I would hence propose to remove this case.

Hmm, I'd disagree for two (imo good) reasons:

1. The case is correct.
2. We might encounter this case in a later test vector of the 🛒🔥 suite, which would make debugging very tedious and difficult. (Who suspects his error to be in a 100% correct solution??)

Lmk what you think!

[niri81]

Hmmmmmm... I think properly seeding your CSPRNG is important for ID generation.

No comment, will forward this to my motoric actors :)

[Sarsum]

While I don't think that the testcase collections will be changed much more, I do agree, that we should include mathematically correct testcases. While some other edgecases are not properly defined and hence not tested in kartfire, this one is at least defined.

[Sarsum]

I think we have "a few" wrong testcases in the repo and it somehow magically works with my current implementation on kartfire. I rechecked the results with the comparator I am using for the sorting as well and got these results:

Testfile                                   | Successful | Failed (C) | Missing (C) | Time Seconds | Failed (T)                           | Missing (T)
-------------------------------------------+------------+------------+-------------+--------------+--------------------------------------+------------
03_action_gfpoly_divmod_p1_01.json         | 255        | 245        | 0           | 0.033        | ['00066', '00295', '00083', '00120'] | None       
03_action_gfpoly_divmod_p2_01.json         | 275        | 225        | 0           | 0.052        | ['00159', '00342', '00015', '00090'] | None
03_action_gfpoly_powmod_p1_01.json         | 66         | 34         | 0           | 0.137        | ['00008', '00003', '00063', '00082'] | None       
03_action_gfpoly_powmod_p2_01.json         | 56         | 44         | 0           | 0.339        | ['00095', '00065', '00059', '00054'] | None

Will need to look into this tomorrow thoroughly...

[niri81]

@tametsi @Sarsum @melo-afk please recheck, one testcase (the one @tametsi complained about) was wrong. ✂️

[melo-afk]

runs on my machine 🚀

[tametsi]

looks good

[niri81]

Tried to resolve the merge conflict (spoiler: only worked partly) :)

[melo-afk]

still runs on my machine 🚀

[Sarsum]

works for me as well