Skip to content

Use a "special values" header before the list of special values for elementwise functions #52

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

Merged
merged 3 commits into from
Oct 19, 2020

Conversation

asmeurer
Copy link
Member

This will make these easier to parse from the test suite, barring something
like #49.

…lementwise functions

This will make these easier to parse from the test suite, barring something
like #49.
@asmeurer
Copy link
Member Author

I will also push up changes here if I find any of these special values to have inconsistent formatting that prevents them from being parsed.

@asmeurer
Copy link
Member Author

Question: atan2 says

-   If `x_i` is `+infinity`, the result is an implementation-dependent approximation to `+π/2` (rounded).
-   If `x_i` is `-infinity`, the result is an implementation-dependent approximation to `-π/2` (rounded).

The other functions that have the "implementation-dependent approximation" rules do not have "(rounded)". Does that mean something different here, or should we change it to be consistent (and if so, should they all say "(rounded)" or should it be removed here)?

asmeurer added a commit to data-apis/array-api-tests that referenced this pull request Oct 15, 2020
@asmeurer
Copy link
Member Author

I pushed some wording inconsistency fixes. With this branch, ./generate_stubs.py in the test suite can now parse all the special values (though it doesn't do anything with them yet). See https://github.com/Quansight/array-api-tests/blob/ad4ac2f7926433c068b751a8d928ab4f8cd56e2e/generate_stubs.py#L111-L138 for all the different types of special values I identified (the last one, REMAINING, is a catchall for the "remaining cases" rules that will have to be handled manually).

@kgryte
Copy link
Contributor

kgryte commented Oct 19, 2020

Re: (rounded). You can update this PR to remove (rounded) in those two instances. We can assume that, as pi/2 cannot be exactly represented in binary floating-point, the approximation is to a rounded pi/2.

@kgryte
Copy link
Contributor

kgryte commented Oct 19, 2020

Re: editorial changes. Most of the changes in this PR are fine. I think my main comment is that I'd caution against relying on, e.g., regular expressions to extract special values and auto-populate test data. This approach is bound to be brittle and requires that spec authors have an accurate mental model when writing specs of how test values are generated. This is likely to prove problematic.

This is also problematic from the standpoint of duplication. Suppose we want the spec to state that the special values for __add__ are the same as add without having to explicitly write them. Instead, we could link to the add spec definition. How would auto-population work there?

Personally, while I recognize the desire to ensure that the specs and tests remain in sync, my sense is that, while perhaps more tedious, the tests should simply include explicit lists of test values and expected results (i.e., no "magic") for each mathematical function having spec'd edge cases. If, and when, a particular function's list of special values and/or behavior changes, then part of the responsibility of editors is to ensure that the two (the spec and tests) remain in sync (i.e., the process is manual).

My sense is that without rigidly codified rules and an extremely well-defined set of rules governing how sentences are organized and constructed, we're likely to encounter issues ensuring that merged spec changes are guaranteed to be reflected in tests.

@rgommers
Copy link
Member

We discussed this in person today. The cautions about brittleness are valid and understood. That said, these language consistency fixes are nice, and we can deal with moving the data for special values of the spec to a more maintainable machine-readable format at a later stage. So let's merge this now.

@rgommers rgommers merged commit c8b63a0 into master Oct 19, 2020
@rgommers rgommers deleted the special-values-formatting branch October 19, 2020 20:30
@rgommers
Copy link
Member

Thanks @asmeurer and @kgryte

cr313 added a commit to cr313/test-array-api that referenced this pull request Apr 19, 2024
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