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

New Caliper Integration #80

Open
wants to merge 35 commits into
base: main
Choose a base branch
from

Conversation

ilumsden
Copy link
Collaborator

@ilumsden ilumsden commented Jan 29, 2024

This PR usurps #51 for adding Caliper annotations to DYAD.

It builds on the recent code in dyad_profiler.h instead of building a separate library for performance annotations.

This is currently a work-in-progress PR. The remaining things to be done before it is ready for merge are:

  • Update CMake to find and link Caliper
  • Test the interface using the work for HiCOMB
  • After HiCOMB, add support for custom attributes (with help from @daboehme where needed)

@ilumsden ilumsden added the enhancement New feature or request label Jan 29, 2024
@ilumsden ilumsden self-assigned this Jan 29, 2024
@ilumsden
Copy link
Collaborator Author

ilumsden commented Jan 29, 2024

This PR is built on top of #79 and #81. So, those PRs should be merged before this one.

@ilumsden
Copy link
Collaborator Author

I can confirm that I am getting Caliper files when running the workflow benchmark with these annotations enabled. I still need to verify that the CCTs I get when loading the data into Thicket look right. Once I've verified that, task 2 in the original comment on this PR will be done.

@ilumsden
Copy link
Collaborator Author

Also, I will switch this PR to ready for review after I know the CCTs are good. Task 3 in the original PR comment can be done later if we want to.

@ilumsden ilumsden marked this pull request as ready for review January 30, 2024 14:58
@ilumsden ilumsden force-pushed the rma_caliper_integration branch 2 times, most recently from 2602f07 to 90b51f6 Compare February 4, 2024 14:32
@ilumsden
Copy link
Collaborator Author

Todos:

  • Split explicit C API into another PR
  • Work with David to finish the last few things for the Caliper annotations

@ilumsden ilumsden force-pushed the rma_caliper_integration branch from 90b51f6 to 020bd29 Compare June 14, 2024 16:20
@ilumsden
Copy link
Collaborator Author

ilumsden commented Jul 9, 2024

@hariharan-devarajan @JaeseungYeom this PR is ready for review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant