Skip to content
This repository was archived by the owner on Jan 12, 2024. It is now read-only.

Conversation

@IrinaYatsenko
Copy link
Contributor

To collect early feedback on the tracer design.

lib/tracer contains implementation of the tracer (the readme file in the folder outlines the requirements and design)
test/unittests/TracerTests.cpp contains C++ unit tests for the tracer
test/QIR-tracer demonstrates how a Q# program can be connected to the tracer via QIR

Copy link
Contributor

@thomashaener thomashaener left a comment

Choose a reason for hiding this comment

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

Thanks a lot, @irinayat-MS, looks good to me!
I've added a few suggestions and comments.

Copy link
Member

@msoeken msoeken left a comment

Choose a reason for hiding this comment

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

I left only a few comments for now.

@IrinaYatsenko IrinaYatsenko force-pushed the irinayat/tracer branch 2 times, most recently from 1c35c3b to f394387 Compare February 6, 2021 00:17
@IrinaYatsenko IrinaYatsenko force-pushed the irinayat/tracer branch 4 times, most recently from 203ddd3 to e8d4f45 Compare February 8, 2021 17:02
@IrinaYatsenko IrinaYatsenko marked this pull request as ready for review February 8, 2021 17:35
Comment on lines 284 to 285
@TargetInstruction("inject_global_barrier")
operation Barrier(id : Int, duration : Int) : Unit
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly here, this looks like a proposal for a new Q# intrinsic operation? If so, would be willing to write an issue on QuantumLibraries so that we can get it scheduled for the next API review? Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bettinaheim, I suspect that eventually we'll need barriers to be in the public interface but I'm not sure of the timeline. For now, I've moved it into the Tracer namespace to allow for experimentation and testing but should we kick off the motions to add it to the libraries as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that would make sense, yeah. No harm in getting the Q# API reviewed in advance so that the approval is ready and in place for whatever timelines work on the C&R side.

Copy link
Contributor

@kuzminrobin kuzminrobin left a comment

Choose a reason for hiding this comment

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

Still reviewing...

Copy link
Contributor

@kuzminrobin kuzminrobin left a comment

Choose a reason for hiding this comment

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

Can you please show the updated README.md?

@kuzminrobin
Copy link
Contributor

Reapproving.

@IrinaYatsenko IrinaYatsenko enabled auto-merge (squash) February 20, 2021 01:53
@IrinaYatsenko IrinaYatsenko merged commit 46ea64d into main Feb 20, 2021
@IrinaYatsenko IrinaYatsenko deleted the irinayat/tracer branch February 20, 2021 03:18
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.

6 participants