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

Fix some doctests that fail for various random seeds #32188

Closed
kliem opened this issue Jul 12, 2021 · 14 comments
Closed

Fix some doctests that fail for various random seeds #32188

kliem opened this issue Jul 12, 2021 · 14 comments

Comments

@kliem
Copy link
Contributor

kliem commented Jul 12, 2021

We fix some doctests or mark them as # not tested that were discovered when testing #29935. This should make the review of #29935 easier.

All the tests failed for some random seed during

sage -t --long --all --random-seed=n

Depends on #32095
Depends on #32185

CC: @orlitzky

Component: doctest framework

Author: Jonathan Kliem

Branch/Commit: 48b8e0b

Reviewer: Michael Orlitzky

Issue created by migration from https://trac.sagemath.org/ticket/32188

@kliem kliem added this to the sage-9.4 milestone Jul 12, 2021
@kliem
Copy link
Contributor Author

kliem commented Jul 12, 2021

New commits:

d471e7efix various doctests for random seeds
430afa9two more doctests in graphs
a11e2d9a few more tests
b9a8e05one more failing test in coding
531f859two more doctests
f975a60a few more fixes for various doctests
cb42bc3two more doctests

@kliem
Copy link
Contributor Author

kliem commented Jul 12, 2021

Commit: cb42bc3

@kliem
Copy link
Contributor Author

kliem commented Jul 12, 2021

@orlitzky
Copy link
Contributor

Dependencies: #32095, #32185

@orlitzky
Copy link
Contributor

Changed commit from cb42bc3 to 36e8666

@orlitzky
Copy link
Contributor

comment:3

I've made three review commits: two just re-enable the tests for #32095 and #32185, and the last one tries to do something more useful with that rank example over GF(2^4)[a].

I have one other question:

sage: while True:
....:     try:
....:         y = Chan(c)
....:         D.decode_to_message(y)
....:     except ZeroDivisionError:
....:         pass

That looks suspicious to me. It's looping until... what? Neither c,D, nor y change inside the loop unless there's some randomness I missed.

Otherwise the rest LGTM.

@orlitzky
Copy link
Contributor

@mkoeppe mkoeppe modified the milestones: sage-9.4, sage-9.5 Aug 9, 2021
@kliem
Copy link
Contributor Author

kliem commented Sep 1, 2021

Changed commit from 36e8666 to 48b8e0b

@kliem
Copy link
Contributor Author

kliem commented Sep 1, 2021

Changed branch from u/mjo/ticket/32188 to u/gh-kliem/ticket/32188

@kliem
Copy link
Contributor Author

kliem commented Sep 1, 2021

New commits:

48b8e0bfix merge conflict

@orlitzky
Copy link
Contributor

orlitzky commented Sep 1, 2021

comment:7

To answer my own question from comment:3, it looks like Chan(c) does something that eliminates the ZeroDivisionError, leading to the DecodingError that we expect:

# We arrive at this point by running
# c = C.random_element(); y = Chan(c); D.decode_to_message(y)
# in a loop until a ZeroDivisionError is thrown.
sage: c                                                                         
(56, 57, 51, 49, 50, 51, 18, 6, 56, 35, 53, 49, 25, 47, 19, 31, 3, 22, 34, 38, 53, 34, 29, 15, 58, 38, 21, 53, 23, 14, 1, 23, 22, 37, 39, 12, 8, 23, 39, 37)
sage: y                                                                         
(56, 22, 51, 36, 50, 10, 18, 6, 56, 35, 53, 49, 58, 56, 53, 31, 3, 22, 34, 3, 53, 34, 38, 15, 0, 38, 21, 0, 46, 3, 1, 23, 43, 42, 39, 12, 8, 23, 33, 37)
sage: D.decode_to_message(y)
...
ZeroDivisionError: inverse of Mod(0, 59) does not exist
sage: y = Chan(c)                                                               
sage: D.decode_to_message(y)
...
DecodingError: Decoding failed because the number of errors exceeded the decoding radius

Regardless... it's doing what it should somehow.

@orlitzky
Copy link
Contributor

orlitzky commented Sep 2, 2021

Reviewer: Michael Orlitzky

@kliem
Copy link
Contributor Author

kliem commented Sep 2, 2021

comment:9

Thank you.

I had somehow forgotten about your question from comment:3.

@vbraun
Copy link
Member

vbraun commented Sep 7, 2021

Changed branch from u/gh-kliem/ticket/32188 to 48b8e0b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants