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

adding roots function for jax frontend #23481

Closed
wants to merge 8 commits into from
Closed

Conversation

dash96
Copy link
Contributor

@dash96 dash96 commented Sep 12, 2023

PR Description

Related Issue

Close #23179

Checklist

  • Did you add a function?
  • Did you add the tests?
  • Did you follow the steps we provided?

Socials:

@github-actions
Copy link
Contributor

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! 🎉👨‍💻

@ivy-leaves ivy-leaves added the JAX Frontend Developing the JAX Frontend, checklist triggered by commenting add_frontend_checklist label Sep 12, 2023
@dash96
Copy link
Contributor Author

dash96 commented Sep 12, 2023

@Ookamice please help me understand this error. Because when I use some random inputs to this function to verify it with jax.roots() it provides the the same results,

fun_name = 'roots', args = ([Array([-1., -1., -1., -1., -1.], dtype=float32)],)
pos = 0, arg = [Array([-1., -1., -1., -1., -1.], dtype=float32)]
msg = '{} requires ndarray or scalar arguments, got {} at position {}.'

def check_arraylike(fun_name: str, *args: Any):
  """Check if all args fit JAX's definition of arraylike."""
  assert isinstance(fun_name, str), f"fun_name must be a string. Got {fun_name}"
  if any(not _arraylike(arg) for arg in args):
    pos, arg = next((i, arg) for i, arg in enumerate(args)
                    if not _arraylike(arg))
    msg = "{} requires ndarray or scalar arguments, got {} at position {}."
  raise TypeError(msg.format(fun_name, type(arg), pos))

E TypeError: roots requires ndarray or scalar arguments, got <class 'list'> at position 0.
E Falsifying example: test_jax_roots(
E on_device='cpu',
E frontend='jax',
E backend_fw='numpy',
E dtype_and_x=(['float32'],
E [array([-1., -1., -1., -1., -1.], dtype=float32)]),
E test_flags=FrontendFunctionTestFlags(
E num_positional_args=0,
E with_out=False,
E inplace=False,
E as_variable=[False],
E native_arrays=[False],
E test_compile=False,
E generate_frontend_arrays=False,
E transpile=False,
E precision_mode=False,
E ),
E fn_tree='ivy.functional.frontends.jax.numpy.roots',
E )
E
E You can reproduce this example by temporarily adding @reproduce_failure('6.82.3', b'AXicY2AAAUYGBjhNKhsIAAGjAAs=') as a decorator on your test case

/opt/fw/jax/jax/_src/numpy/util.py:328: TypeError

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.

Conventional Commit PR Title

In order to be considered for merging, the pull request title must match the specification in conventional commits. You can edit the title in order for this check to pass.
Most often, our PR titles are something like one of these:

  • docs: correct typo in README
  • feat: implement dark mode"
  • fix: correct remove button behavior

Linting Errors

  • Found type "null", must be one of "feat","fix","docs","style","refactor","perf","test","build","ci","chore","revert"
  • No subject found

@dash96 dash96 changed the title adding roots function for jax frontend test: adding roots function for jax frontend Sep 13, 2023
@dash96
Copy link
Contributor Author

dash96 commented Sep 15, 2023

How can I work with dtype errors. During all the tests I am getting the right answers but not the right dtypes. When ground truth is complex64 I am getting complex128 and when I hard code the dtype="complex64" in the function, for the same test cases ground truth is changing to complex128

@ivy-seed ivy-seed assigned ZoeCD and unassigned Ookamice Sep 23, 2023
@dash96
Copy link
Contributor Author

dash96 commented Sep 27, 2023

Hello! @ZoeCD all the tests are passing in my system. Could you please help me understand what shall I do next.

@dash96 dash96 changed the title test: adding roots function for jax frontend adding roots function for jax frontend Oct 4, 2023
@dash96
Copy link
Contributor Author

dash96 commented Oct 11, 2023

Hello @DecFox Please help me in merging this PR all the tests are passing from my end

@dash96
Copy link
Contributor Author

dash96 commented Oct 27, 2023

Hello @hirwa-nshuti could you please help me merge this function it has been here since a month.

atol=1e-05,
rtol=1e-03,
)
ret, frontend_ret = call()
Copy link
Contributor

@Ishticode Ishticode Mar 3, 2024

Choose a reason for hiding this comment

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

unless test_values=False, the test_frontend_function doesn't return the values. In addition this call() should be made outside the the definition of call. Tbh we don't even need to define it as a call. But thats fine.
Once this is updated pls request my review. Thnx

@ivy-seed ivy-seed added the Stale label Mar 11, 2024
@ivy-seed
Copy link

This PR has been labelled as stale because it has been inactive for more than 7 days. If you would like to continue working on this PR, then please add another comment or this PR will be closed in 7 days.

@ivy-seed ivy-seed closed this Mar 18, 2024
@ivy-seed
Copy link

This PR has been closed because it has been marked as stale for more than 7 days with no activity.

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 Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

roots
9 participants