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.

Sign up for GitHub

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

Jump to bottom

[keras/benchmarks/benchmark_util.py] Use var rather than string literal for is None checks on measure_performance #17980

Merged
merged 1 commit into from
May 16, 2023
Merged

[keras/benchmarks/benchmark_util.py] Use var rather than string literal for is None checks on measure_performance #17980

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions keras/benchmarks/benchmark_util.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,13 @@ def measure_performance(
ValueError: If `x` is none or if `optimizer` is not provided or
if `loss` is not provided or if `num_gpus` is negative.
"""
if "x" is None:
if x is None:
raise ValueError("Input data is required.")
if "optimizer" is None:
elif optimizer is None:
Copy link
Contributor

@Frightera Frightera Apr 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @SamuelMarks,

Just curiosity, why did you switch into elif instead of if? I think keeping them independent might be a better practice, i.e maintain the if statements.

P.S: Don't take this as an official-review.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Frightera Because it's more efficient:

$ python -m timeit 'a="foo"' 'if a == "foo":' '   a += "bar"' 'if a == "can":' '   a += "can"'
5000000 loops, best of 5: 51.9 nsec per loop
$ python -m timeit 'a="foo"' 'if a == "foo":' '   a += "bar"' 'elif a == "can":' '   a += "can"'
10000000 loops, best of 5: 39.6 nsec per loop

Also it's more logical. The compiler probably optimises the branches better, and it's clearer to read.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I think the speed difference is because of every if statement is executed in your example whereas in here there are ValueErrors which will cause not to execute other statements, probably the performance difference will be negligible.

raise ValueError("Optimizer is required.")
if "loss" is None:
elif loss is None:
raise ValueError("Loss function is required.")
if num_gpus < 0:
elif num_gpus < 0:
raise ValueError("`num_gpus` cannot be negative")

# TODO(xingyulong): we will add tfds support later and
Expand Down