Skip to content

Fix non-memcpy benchmark path saving raw timestamps instead of cycle counts - #73

Open
ycmath wants to merge 1 commit into
spcl:mainfrom
ycmath:fix-noncmemcpy-perf-cycles
Open

ycmath wants to merge 1 commit into
spcl:mainfrom
ycmath:fix-noncmemcpy-perf-cycles

Conversation

@ycmath

@ycmath ycmath commented Aug 26, 2026

Copy link
Copy Markdown

In the non-memcpy benchmark path, Program.run calls copy_back_benchmark_data, which returns a (cycle_start, cycle_stop) tuple of absolute timestamps, and binds the tuple to cycle_counts. Consequently:

  • perf_cycles.npy stores the two stacked timestamp arrays instead of elapsed cycle counts, and
  • the printed "Cycle count" summary is computed from absolute timestamps.

The memcpy path already computes the difference correctly. This changes the call to copy_back_benchmark_cycles, which returns cycle_stop - cycle_start, making both paths consistent.

🤖 Generated with Claude Code

…counts

The non-memcpy benchmark path called copy_back_benchmark_data, which
returns a (cycle_start, cycle_stop) tuple of absolute timestamps, and
bound it to cycle_counts. As a result perf_cycles.npy stored the two
stacked timestamp arrays rather than elapsed cycles, and the printed
"Cycle count" was computed from absolute timestamps. Call
copy_back_benchmark_cycles, which returns the stop-start difference,
matching the memcpy path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@tbennun
tbennun self-requested a review September 15, 2026 15:22

@tbennun tbennun left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM!

@ycmath

ycmath commented Sep 21, 2026

Copy link
Copy Markdown
Author

Thanks for the review! For the record, the test-csl failure here is a fork-PR limitation rather than this change: the job fetches the SDK with wget "${{ secrets.CSL_SDK_URL }}", and GitHub does not pass repository secrets to workflows triggered by PRs from forks, so the URL comes through empty (log: wget -q -O sdk-1.4.0.tar.gz "" → exit 1; the Linux-deps cache had also expired). test-pytest passes, and main itself is green. Same constraint applies to #72.

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.

2 participants