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

Remove tf.function in gradient operator to avoid multiple useless retracings #53

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

cofri
Copy link
Contributor

@cofri cofri commented Jun 9, 2023

The tf.function decorator on the gradient function operator was slowing down because of multiple retracings. Removing it speeds up the computation.

Adding tf.function on ODIN input_perturbation() function was the fastest option but input_perturbation() would not be framework-agnostic anymore.

As a temporary hotfix, we remove @tf.function decorator from gradient method to prevent useless raytracing. We will work on a more long-term solution in a future PR (see issue #52) .

The tf.function decorator on the gradient function was slowing down because of
multiple retracings. Removing it speeds up the computation.
Adding tf.function on ODIN `input_perturbation()` function was the fastest
option but input_perturbation() is not framework-agnostic anymore.
@github-actions
Copy link

github-actions bot commented Jun 9, 2023

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
1613 1453 90% 70% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
oodeel/utils/tf_operator.py 96% 🟢
TOTAL 96% 🟢

updated for commit: 665bb1b by action🐍

@cofri cofri requested review from paulnovello and y-prudent June 9, 2023 15:36
@cofri cofri marked this pull request as ready for review June 9, 2023 15:40
Copy link
Member

@paulnovello paulnovello left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Member

@y-prudent y-prudent left a comment

Choose a reason for hiding this comment

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

LGTM!

@y-prudent y-prudent merged commit ebd60e6 into master Jun 9, 2023
@cofri cofri deleted the fix/tf_gradient_too_slow branch August 31, 2023 13:04
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