-
-
Notifications
You must be signed in to change notification settings - Fork 768
Eliminate remaining 32- and 64-bit doctest output tags #41540
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
base: develop
Are you sure you want to change the base?
Conversation
Rewrite one test that depends on the bitness of the machine to use the 32_bit feature tag rather than the non-standard output tags.
Rewrite one test in this file to use the 32_bit feature rather than the (nonstandard) bitness tags on the output.
We are testing two separate cases for the quotient x/y in a multivariate polynomial ring, but both answers are wrong. Rather than combine the two cases, I think we can stop testing this bug.
Move the bitness tags from the output to the input in one example that returns slightly different answers on 32- and 64-bit systems.
One test in this file has special cases for 32- and 64-bit systems, but is marked "# not tested" anyway. Since we are not testing it (it takes several minutes to run), we delete the test to avoid the need for special cases in the output.
Use "# needs 32_bit" on the input rather than "# 32-bit" on the output, as the latter is nonstandard.
Use the appropriate "needs" tag on the input line rather than tagging the output with the bitness (which is non-standard).
Tag the input with the bitness rather than the output; the latter is nonstandard.
The one example for arb_to_mpfi() has special cases for the machine bitness, but it's not a great example anyway. We first provide a better example (showing that we've actually converted arb something to mpfi something), and then rewrite the old example to tag the input with the bitness rather than the output.
There's a doctest in this file that overflows on both 32- and 64-bit systems with slightly different output, but we don't need to test that (differing) output. We can combine the two cases with ellipses.
One test in this file has special cases for the output on 32- and 64-bit systems, but I'm fairly sure that with two coefficients on the order of e-19 and an abs tol of 1e-14, the remaining coefficient of 0.759316500288427 is the only one tested -- and it's the same regardless of bitness.
Most of the 32- and 64-bit tags in these files are for outputs that can only be compared as strings anyway, because there's no easy way to input them literally. For those we replace the "32-bit" and "64-bit" tags on the output with "needs 32_bit" and "needs !32_bit" on the input. This is a little bit more standard.
Instead of recommending the 32-bit and 64-bit output tags (which are nonstandard), suggest the 32_bit feature.
Make one example follow the recommendation that follows it.
These will be removed shortly.
These have been excised from the sage library.
|
Documentation preview for this PR (built with commit 92afcf2; changes) is ready! 🎉 |
|
The tests look OK on CI, too, but I made a few non-trivial edits that need to be checked on a 32-bit system. |
|
Can you check this also on 32-bit OS ? |
I don't have a 32-bit machine available but I've CCed two people who might. |
tobiasdiez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is a nice idea! Volkers bots will test a 32bit system, so let's try this.
Our
# 32-bitand# 64bittags, namely that they must annotate the output lines and not the input line, are non-standard. We have replacements that work on the input lines in# needs 32_bitor# needs !32_bit, but in some cases we can handle both cases simultaneously (which is preferable, in my opinion).This PR replaces all uses of the old tags, updates the documentation to suggest
# needs 32_bit, and removes support for the old tags from the doctest runner. The latter is a backwards-incompatible change, but it cannot break user code, only custom tests. If we were to make the old-style tags raise deprecation warnings, exactly the same tests would break, so it does not seem worthwhile to deprecate and remove later.This is a follow-up to #41468, and supersedes #40238.
The tests pass for me on a 64-bit system, but I don't have a 32-bit one to try with.