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

Adds Performance Counter and user defined Scale Factor options to Python Speech Sample #6650

Closed

Conversation

kenkoyanagi
Copy link
Contributor

Three new cmd line arguments:

  • -pc = performance counter
  • -sf = scale factor
  • -a/--arch = either CORE or ATOM

-a/--arch is a complimentary argument for when -pc is used to allow for correct interpretation of the performance counter values returned.

@kenkoyanagi kenkoyanagi requested a review from a team as a code owner July 14, 2021 21:58
@kenkoyanagi kenkoyanagi requested a review from a team July 14, 2021 21:58
@openvino-pushbot openvino-pushbot added ExternalPR External contributor category: docs OpenVINO documentation category: Python API OpenVINO Python bindings labels Jul 14, 2021
Comment on lines +152 to +154
if args.performance_counter:
plugin_config['PERF_COUNT'] = 'YES'

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the option enabled only for GNA device?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe this aligns with the current C++ implementation.

Comment on lines 251 to 268
if args.performance_counter:
pc = perf_counters[i]
total_cycles = int(pc["1.1 Total scoring time in HW"]["real_time"])
stall_cycles = int(pc["1.2 Stall scoring time in HW"]["real_time"])
active_cycles = total_cycles - stall_cycles
frequency = 10**6
if args.arch == "CORE":
frequency *= 400
else:
frequency *= 200
total_inference_time = total_cycles/frequency
active_time = active_cycles/frequency
stall_time = stall_cycles/frequency
print("\nPerformance Statistics of GNA Hardware")
print(" Total Inference Time: " + str(total_inference_time * 1000) + " ms")
print(" Active Time: " + str(active_time * 1000) + " ms")
print(" Stall Time: " + str(stall_time * 1000) + " ms\n")

Copy link
Contributor

Choose a reason for hiding this comment

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

It works only on harware GNA, on software it always shows 0.0ms.
image

@dorloff Do we need to get a per-layer report as in C++ version?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it's expected that it's 0.0 on SW emulation mode.
I think we can have a per-layer report, but don't need to do it in this PR.



if args.arch:
if args.arch not in ("CORE", "ARCH"):
Copy link
Contributor

Choose a reason for hiding this comment

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

combine two ifs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed - handling in argparser choices as mentioned below. :)


if args.arch:
if args.arch not in ("CORE", "ARCH"):
log.error('The architectuer argument only accepts CORE or ARCH')
Copy link
Contributor

@generalova-kate generalova-kate Jul 15, 2021

Choose a reason for hiding this comment

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

you can add such a check via argparser choices option, it will be tracked automatically

total_inference_time = total_cycles/frequency
active_time = active_cycles/frequency
stall_time = stall_cycles/frequency
print("\nPerformance Statistics of GNA Hardware")
Copy link
Contributor

Choose a reason for hiding this comment

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

please use logger

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change made

if args.arch == "CORE":
frequency *= 400
else:
frequency *= 200
Copy link
Contributor

Choose a reason for hiding this comment

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

define magic numbers via global variables and add comments

@kenkoyanagi
Copy link
Contributor Author

@generalova-kate - thanks for your feedback! Made the changes requested.

@kenkoyanagi
Copy link
Contributor Author

@generalova-kate - does this PR look ready to merge?

@kenkoyanagi
Copy link
Contributor Author

@dorloff - fyi in case you want to close on this

@dpigasin dpigasin closed this Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: docs OpenVINO documentation category: Python API OpenVINO Python bindings ExternalPR External contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants