Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

skip 3 steps in benchmarking #75

Merged
merged 2 commits into from
Feb 8, 2024
Merged

skip 3 steps in benchmarking #75

merged 2 commits into from
Feb 8, 2024

Conversation

crazydemo
Copy link
Contributor

This PR is an alternative to the PR#73

for i, x in enumerate(test_loader):
while True:
step += 1
sample = next(iter(test_loader))
Copy link
Contributor

Choose a reason for hiding this comment

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

This will force us to always use the first batch. Why don't you just increase min_batches? How many do you need? What combinations of parameters is the problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you are right, this will force us to always use the first batch, which is not I want to do in this PR. I will revert this part. I can just increase min_batches to get stable performance data.

@crazydemo crazydemo force-pushed the zhangyan/fix_perf2 branch 2 times, most recently from 2103a14 to 3fa7177 Compare February 7, 2024 15:32
@@ -415,7 +417,7 @@ def inference(self, backend: Backend):
y = self.net(x)
else:
y = self.net(x)

if i < 3: continue
Copy link
Contributor Author

@crazydemo crazydemo Feb 7, 2024

Choose a reason for hiding this comment

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

But the point of this PR is that, current warmup seems not working. According to the onednn verbose, the first 3 steps in benchmarking (duration_s) period show quite poor performance. I think this can be the same issue with Issue#66. @Egor-Krivov Will you follow up Issue#66? Or may I just skip the first 3 steps in benchmarking period?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have problem with this part. You can skip first 3 steps

@Egor-Krivov
Copy link
Contributor

Let me test this code and then I can merge

@crazydemo crazydemo requested a review from ZhennanQin February 7, 2024 16:18
@Egor-Krivov Egor-Krivov merged commit 408045e into main Feb 8, 2024
10 checks passed
@Egor-Krivov Egor-Krivov deleted the zhangyan/fix_perf2 branch February 8, 2024 13:11
Egor-Krivov pushed a commit that referenced this pull request Feb 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants