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

Log1p #16521

Merged
merged 53 commits into from
Jul 25, 2023
Merged

Log1p #16521

merged 53 commits into from
Jul 25, 2023

Conversation

MuhammadNizamani
Copy link
Contributor

@MuhammadNizamani MuhammadNizamani commented Jun 11, 2023

Adding log1p function paddle.tensor.tensor.Tensor from issue Closed #16519

@MuhammadNizamani
Copy link
Contributor Author

@zaeemansari70 sir waiting for your review.

Copy link
Contributor

@zaeemansari70 zaeemansari70 left a comment

Choose a reason for hiding this comment

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

Hi @MuhammadNizamani,
Sorry for the delay in the review process. Thanks a lot for your patience. 🙂

I tested your changes locally they are failing for torch, with
AssertionError: the results from backend torch and ground truth framework paddle do not match E inf!=87.49822998046875
The array being passed has the value of 1.0000001e+38, maybe try looking into applying a small safety factor and see if it fixes it!

Let me know if you have any questions 🙂
Thanks!

@MuhammadNizamani
Copy link
Contributor Author

@zaeemansari70 can you tell how can I test for torch.
and another question this paddle is tensor base then why you are runing it in torch.

Copy link
Contributor

@zaeemansari70 zaeemansari70 left a comment

Choose a reason for hiding this comment

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

Hi @MuhammadNizamani,
Just for the context, when we run a frontend function, our ground_truth is the actual frontend function from the original framework and is tested against every backend which we have in our Ivy for that specific function.
There can be an issue in the backends sometimes due to which the tests fail but this time it is particularly related to the frontends as the array which is being passed inside the function is of a huge number which can be fixed by a safety factor in your test function.

I've also added the falsifying example so you can test on it!

E           AssertionError:  the results from backend torch and ground truth framework paddle do not match
E            inf!=87.49822998046875 
E           
E           
E           Falsifying example: test_paddle_log1p(
E               frontend='paddle',
E               on_device='cpu',
E               dtype_and_x=(['float32'], [array(1.0000001e+38, dtype=float32)]),
E               frontend_method_data=FrontendMethodData(ivy_init_module=<module 'ivy.functional.frontends.paddle' from '/workspaces/ivy/ivy/functional/frontends/paddle/__init__.py'>, framework_init_module=<module 'paddle' from '/opt/fw/paddle/paddle/__init__.py'>, init_name='to_tensor', method_name='log1p'),
E               init_flags=FrontendMethodTestFlags(
E                   num_positional_args=1,
E                   as_variable=[False],
E                   native_arrays=[False],
E               ),
E               method_flags=FrontendMethodTestFlags(
E                   num_positional_args=0,
E                   as_variable=[False],
E                   native_arrays=[False],
E               ),
E           )
E           
E           You can reproduce this example by temporarily adding @reproduce_failure('6.78.3', b'AAEAAQAAAIfiztMwAAAAAAAAAAAAAAA=') as a decorator on your test case

Thanks! 🙂

@ivy-leaves ivy-leaves added the Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist label Jun 19, 2023
@ivy-leaves
Copy link

If you are working on an open task, please edit the PR description to link to the issue you've created.

For more information, please check ToDo List Issues Guide.

Thank you 🤗

Copy link
Contributor

@zaeemansari70 zaeemansari70 left a comment

Choose a reason for hiding this comment

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

The changes look good to me 👍
Kindly resolve the conflicts so I can merge the PR. 🙂

@MuhammadNizamani
Copy link
Contributor Author

On it

@MuhammadNizamani
Copy link
Contributor Author

@zaeemansari70 merge conflict are resolve

@zaeemansari70
Copy link
Contributor

Hi @MuhammadNizamani ,

The conflicts are still being showed here 🤔

@MuhammadNizamani
Copy link
Contributor Author

@zaeemansari70 sir I usually resolve the merge conflicts in this PR.

Copy link
Contributor

@zaeemansari70 zaeemansari70 left a comment

Choose a reason for hiding this comment

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

There was a small change which needed to be made, now your PR looks good to be merged, thanks! 🙂

@zaeemansari70 zaeemansari70 merged commit e4df814 into ivy-llc:master Jul 25, 2023
@MuhammadNizamani MuhammadNizamani deleted the log1p branch July 26, 2023 05:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Paddle Frontend Developing the Paddle Frontend, checklist triggered by commenting add_frontend_checklist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

log1p
3 participants