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

feat: add script to automatically infer (un)supported dtypes #23003

Closed
wants to merge 34 commits into from
Closed

feat: add script to automatically infer (un)supported dtypes #23003

wants to merge 34 commits into from

Conversation

jshepherd01
Copy link
Contributor

@jshepherd01 jshepherd01 commented Sep 4, 2023

PR Description

Adds a (work in progress) script that can be pointed at any frontend/ or backend/ file and will automatically add the with_(un)supported_(device_and_)dtypes decorators to every function it finds there. It does this by mocking helpers.get_dtypes and helpers.test_function, then running the test for the relevant function once with a minimal example of each dtype and inspecting any generated errors.

Currently, the new script file contains several TODO comments, which I'm currently working through. I'm also planning to prune down the mock implementation of test_function, since I'm fairly sure it doesn't really need to e.g. handle the out argument for the sake of this script

Related Issue

The supported dtypes decorators were quite messy and full of shortcuts (like the test uses get_dtypes('float') so the with_unsupported_dtypes decorator only considers floats) or incorrectness (like bfloat16 being marked as unsupported because the numpy-based testing pipeline threw an error), and also they had to be updated manually when a new framework version came out which would lead to newly-supported dtypes being missed.

README

To call the script on e.g. the numpy backend's activation functions you'd do

python3 determine_dtypes.py ivy/functional/backends/numpy/activations.py -v

you can also pass in multiple files (just give it multiple file paths) or pass it directories and it will recursively search for files. It then gets the frontend or backend name from the file path, searches the file for function definitions (of the form def func(, and it ignores private functions), and also finds the relevant test file and test function for each one according to our naming conventions.

For each file, it mocks the helpers.get_dtypes function to return a single dtype (iterating over ivy.valid_dtypes) and then runs the test function, which will call a mock of either test_function or test_frontend_function. That mock runs the minimal amount of code needed to produce arguments for either the relevant ivy function (for backend functions) or the relevant function in the gt framework (for frontend functions). It calls it wrapped in a try block, and then there's some error handling with string matching on the error message to identify whether errors were dtype-related or not.

Once it's got all the dtype support information it needs, it then reads the existing dtype decorators from the file, combines what they say with the information from the tests it's just run, and writes the new decorators to the file. It also changes the imports if need be.

The general flow of what it does is in the run_dtype_setter function (line 1563), and a lot of the complicated stuff is handled by the FileTester classes (lines 700 for base, 1164 for backend, and 1223 for frontend).

The biggest limitations currently are that it doesn't work for methods and that there seems to be a bug with the handling of non-cpu devices in the frontends. Fixing the former probably means classifying the functions as either regular or method functions when they're read from the file, then modifying some of the other logic to use that information; fixing the latter would probably involve changing FrontendFileTester.setup_test and mock_test_frontend_function.

Since I've now finished my internship I probably won't be able to do major work on this (i.e. new features or very involved fixes) but I'm happy to do minor bug fixes or to explain parts of the code if needed.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 4, 2023

Thanks for contributing to Ivy! 😊👏
Here are some of the important points from our Contributing Guidelines 📝:
1. Feel free to ignore the run_tests (1), run_tests (2), … jobs, and only look at the display_test_results job. 👀 It contains the following two sections:
- Combined Test Results: This shows the results of all the ivy tests that ran on the PR. ✔️
- New Failures Introduced: This lists the tests that are passing on main, but fail on the PR Fork. Please try to make sure that there are no such tests. 💪
2. The lint / Check formatting / check-formatting tests check for the formatting of your code. 📜 If it fails, please check the exact error message in the logs and fix the same. ⚠️🔧
3. Finally, the test-docstrings / run-docstring-tests check for the changes made in docstrings of the functions. This may be skipped, as well. 📚
Happy coding! 🎉👨‍💻

It scans for existing decorators before each function, removes them if
they exist, and then writes the new ones in their place.

This also fixes a formatting issue in the decorators themselves, which
previously had too many commas
verbosity level replaces warnings. unsure support flags define how to
handle cases where the support for a dtype is uncertain (i.e. when the
function raises an error message that isn't recognised). safety mode
prevents the code from writing to file in order to make debugging
easier.
Also modify code to address all edge cases discovered when running on
jax functions.
Modify some test functions to remove use of .filter from test case
generation.
Modify some functions to fix type hints
Refactor mocked test_function to reduce complexity and dependency on
helpers module
Suppress printing of <traceback> objects from failed tests
Skip entire function if all dtypes were skipped for it
Original dtype decorators can now inform the script of what to do if a
dtype or device has to be skipped from testing (it's assumed that the
original decorator was correct about that dtype and it's left unchanged)
Some functions roll their own tests rather than using test_function or
test_frontend_function, which means this script is unable to hook into
them properly. Those functions are now skipped
Also modify test_general to put the imports needed only for a single
test inside that test itself (which also fixes that test) in order to
allow importing of that file into the auto_dtypes script when the torch
backend is enabled (and most of the uints don't exist, which was causing
errors importing the jax backend
If the functions in question already have decorators describing previous
versions of the framework, the script keeps that information in the
decorators.

NB it currently assumes that the current framework version that it's
running with is the most recent version
@jshepherd01 jshepherd01 marked this pull request as ready for review September 15, 2023 20:14
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Compliance Checks

Thank you for your Pull Request! We have run several checks on this pull request in order to make sure it's suitable for merging into this project. The results are listed in the following section.

Issue Reference

In order to be considered for merging, the pull request description must refer to a specific issue number. This is described in our contributing guide and our PR template.
This check is looking for a phrase similar to: "Fixes #XYZ" or "Resolves #XYZ" where XYZ is the issue number that this PR is meant to address.

Copy link
Contributor

@vedpatwardhan vedpatwardhan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me on a high-level, @RickSanchezStoic could you please test this out to see if any changes are required? Ideally we'd want to be able to directly plug it into one of our workflows which would get triggered on every release to update the decorators. Thanks 😄

@ivy-leaves ivy-leaves added PaddlePaddle Backend Developing the Paddle Paddle Backend. Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist PyTorch Frontend Developing the PyTorch Frontend, checklist triggered by commenting add_frontend_checklist JAX Frontend Developing the JAX Frontend, checklist triggered by commenting add_frontend_checklist labels Oct 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 9, 2023

Thank you for this PR, here is the CI results:


This pull request does not result in any additional test failures. Congratulations!

@jshepherd01 jshepherd01 closed this by deleting the head repository Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JAX Frontend Developing the JAX Frontend, checklist triggered by commenting add_frontend_checklist Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist PaddlePaddle Backend Developing the Paddle Paddle Backend. PyTorch Frontend Developing the PyTorch Frontend, checklist triggered by commenting add_frontend_checklist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants