-
Notifications
You must be signed in to change notification settings - Fork 173
QIR unit tests for partial applications #652
Conversation
|
/AzurePipelines run |
|
No pipelines are associated with this pull request. |
| indices[1] = this.CurrentContext.CreateConstant(i + 1); | ||
| Value ptr = this.CurrentBuilder.GetElementPtr(tupleTypeRef, asStructPointer, indices); | ||
| #pragma warning disable CS0618 // Type or member is obsolete -- computing the type that ptr points to is tricky, so we just rely on it's .NativeType | ||
| var subVal = this.CurrentBuilder.Load(ptr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is Load obsolete? Why are we using something obsolete here?
| qirTest false "TestBigInts" | ||
|
|
||
| [<Fact>] | ||
| let ``QIR controlled partial applications`` () = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for the sake of a decent test coverage it would be nice to also have a test with some adjoint and controlled adjoint applications.
| @EntryPoint() | ||
| operation TestPartials () : Bool | ||
| { | ||
| let rotate = Rz(0.25, _); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to add a couple more scenarios here; e.g. for an operation that usually take a nested tuple Op(a, (b, c)) I'd test the cases Op(, (b, c)), Op(a, (, c)), Op(a, (,)), and Op (a, _).
| using (qb = Qubit()) | ||
| { | ||
| rotate(qb); | ||
| unrotate(qb); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would also be good to check that partial application after functor application works as expected, in particular, I'd check that both Controlled Op(, (a, (b, c))) and Controlled Op(qs, (a, (, c))) work as expected.
| { | ||
| using ((ctrls, qb) = (Qubit[2], Qubit())) | ||
| { | ||
| ck2(ctrls, qb); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An edge case that I think is still worth covering for the sake of a good test coverage is invoking the controlled specialization for an operation that takes a Unit argument.
|
Incorporated into #764. |
Adds some unit tests for partial applications and %Callable values