Add Type1 target package and simulator support

[swernli]

This change adds the Type1 target package for Q# targets that support a limited number of control operations. The change includes support for simulating a Type1 package and tests that verify unitary gate behavior for that package. This part of the ongoing work described in #249.

[swernli]

@bettinaheim @cgranade Something that is worth calling out in these changes is the introduction of Microsoft.Quantum.Intrinsic.ApplyUncontrolledH and similar operations for the native gates supported in Type1. Initially I'd hoped to have these marked as internal so that we wouldn't have to fully document them, but in order for a body intrinsic operation to be implemented outside of its own binary it cannot be marked as internal. Given that two of our goals are a) keeping the target packages as pure Q#, and b) allowing external authors to implement simulators in their own repos without always having to fork this one, there isn't a good way around the requirement that intrinsic callables be public. I did my best to document these appropriately so that we can have a chance to accept them as-is. But I'm certainly open to feedback and ideas regarding their identifiers, namespaces, etc.

By way of background, the need to introduce these was that the target gate set only supports singly controlled X and singly controlled Z, and no other controlled operations. So anywhere that our API has support for the Controlled specialization that accepts arbitrary number of controls, it had to be replaced with a decomposition on top of singly controlled variants.

[cgranade]

Only taken a rough skim so far, but left what comments I have so far; I did have one quick question, though:

Initially I'd hoped to have these marked as internal so that we wouldn't have to fully document them, but in order for a body intrinsic operation to be implemented outside of its own binary it cannot be marked as internal.

Is that a limitation on the Q# source, or the generated C# source? If the latter, is that the case even with the InternalsVisibleTo C# attribute?

[cgranade]

I'd be curious how much this and the MCX call above help performance compared with decomposing at the Q# level and then calling into the lower-level simulator functions.

[swernli]

We are decomposing at the Q# level as much as possible right now, and that's part of what made these necessary. Because the Controlled specialization allows for any number of controls, I need to have a callable that is specifically the singly controlled gate and no Controlled specialization. Otherwise it won't match what the target architecture actually supports. It makes sense from a pure instruction translation standpoint, but it looks very odd and redundant when layered on top of our simulator that supports more than this.

[swernli]

To answer that a slightly different way, the Type1 package captures targeting a gate set is that is significantly more constrained than the one exposed by our simulator. As a result, implementing these limited intrinsics on top of our simulator requires using these C# classes to act as a shim that exposes less functionality. It's essentially the opposite of optimization, and it shows: simulating certain operations using this Type1 package are noticeably slower because it will use a complex decomposition, possibly including additional qubits and entanglers, instead of just calling directly into an optimized simulator API.

[cgranade]

This looks to use an additional spare qubit when measuring a multi-qubit Pauli operator; that's completely valid, but there's a tradeoff between that and finding the right Clifford to map back to a single-qubit measurement, then calling the measurement within that Clifford.

[swernli]

Good point! The hope here is that as we find alternative decompositions for certain operations, we can introduce those here and allow users to select between them by choosing different targeting packages. How we can do that in a way that doesn't cause an explosion in the number of targeting packages is another question though...

[swernli]

Initially I'd hoped to have these marked as internal so that we wouldn't have to fully document them, but in order for a body intrinsic operation to be implemented outside of its own binary it cannot be marked as internal.

Is that a limitation on the Q# source, or the generated C# source? If the latter, is that the case even with the InternalsVisibleTo C# attribute?

It is just a C# limitation, but even using InternalsVisibleTo only solves the problem for me and not future authors. Because InternalsVisibleTo requires putting the binary name and signing key on the class that defines the internal, it would mean putting on the target package the names of all binaries that implement it. For binaries in this repo that's easy enough, but then it becomes much harder for someone working in their own repo on a custom simulator or simulator extension to implement the target package intrinsics. Ideally, we wanted to land at a design that would allow for extensibility or custom implementation in a library without having to fork this repo or commit directly to it. But for now that seems to put us in the awkward position of needing to make public each of these arguably internal, helper callables, and then we get stuck supporting them.

[swernli]

@cgranade @bettinaheim I suppose I could use the InternalsVisibleTo as a temporary work around, where the changes in the feature branch do not introduce any new public APIs at the cost of having the target packages only be implementable in this repo for now, and then file a separate issue covering the need to allow implementation of a target package from another repo. That would punt the problem farther down the road, and we'd need to decide if we want to allow merging the feature branch into master with that work still pending or if we want to have a solution for this problem before accepting the target package feature into master.

[cgranade]

@cgranade @bettinaheim I suppose I could use the InternalsVisibleTo as a temporary work around, where the changes in the feature branch do not introduce any new public APIs at the cost of having the target packages only be implementable in this repo for now, and then file a separate issue covering the need to allow implementation of a target package from another repo. That would punt the problem farther down the road, and we'd need to decide if we want to allow merging the feature branch into master with that work still pending or if we want to have a solution for this problem before accepting the target package feature into master.

I agree that approach would kick the can down the road, but I honestly think that may be for the best in this case. Introducing new public Q# APIs comes at a large cost in terms of long-term maintenance, such that kicking the can gives us a bit of an opportunity to come up with an approach that doesn't commit us longer-term. That's just my 2¢, though, so please take with an appropriate grain of salt (apologies for mixed metaphors).

[bamarsha]

But for now that seems to put us in the awkward position of needing to make public each of these arguably internal, helper callables, and then we get stuck supporting them.

Since they're intended to be implemented by simulators, it seems to me like they are actually part of the public API? Is the issue only that it shouldn't be exposed to Q# code, or that it shouldn't be exposed at all (in Q# or .NET)?

[cgranade]

But for now that seems to put us in the awkward position of needing to make public each of these arguably internal, helper callables, and then we get stuck supporting them.

Since they're intended to be implemented by simulators, it seems to me like they are actually part of the public API? Is the issue only that it shouldn't be exposed to Q# code, or that it shouldn't be exposed at all (in Q# or .NET)?

I think part of the problem is that they are part of the public C# API, but shouldn't be part of the public Q# API; from the perspective of a quantum program, these APIs are an implementation detail internal to the simulation runtime, if I understand correctly.

[bamarsha]

I see, so we want the operation to be internal in Q#, but for C# generation to generate a public class. That is technically possible to do if we change C# generation, but it seems like there should be a cleaner way.

[cgranade]

I see, so we want the operation to be internal in Q#, but for C# generation to generate a public class. That is technically possible to do if we change C# generation, but it seems like there should be a cleaner way.

Agreed, there should be; hence why I tend to lean towards kicking the can whilst we brainstorm on and design that better way, rather than commiting to new public APIs in lieu of that.

[swernli]

Agreed on all points, thanks @cgranade and @SamarSha! I'll make the update in this PR to switch these new additions to internal and add this specific issue to the checklist in #249 so we can review it before the feature branch gets merged. For now, I'll treat any and all callabes that would be API additions as internal to avoid any new public API surface, and then we can debate the merits of exposing and fully documenting them on a case by case basis at a later date.

[swernli]

Ok, this seems like it should be ready for merge into the feature branch. Please let me know if there are any other things I should address in this PR and/or track in the overall issue for this feature!

[swernli]

This is once again building and passing the tests :). I'd like to try merging it by EOD.

[swernli]

@SamarSha @cgranade @alan-geller @bettinaheim Any chance I could get sign off on this so I can get it merged into the feature branch? Thanks!

[bamarsha]

Or Length(paulis) == 0? Does the qubits[0] reference below fail in that case?

[swernli]

Thankfully no, because this utility is only called by Exp in ExpFromExpUtil and ExpFrac in ExpFracFromExpUtil with a check:

            if (Length(paulis) != 0) {

or

            let (newPaulis, newQubits) = RemovePauliI(paulis, qubits);

            if (Length(newPaulis) != 0) {
                ExpUtil(newPaulis, theta , newQubits, R(_, -2.0 * theta, _));
            }
            else {
                ApplyGlobalPhase(theta);
            }

Where RemovePauliI correctly handles empty lists.

[bamarsha]

Maybe a quick comment mentioning this would be good, since it's potentially a gotcha if another place uses this operation without checking for length == 0?

[bamarsha]

Or Length(paulis) == 0?

[swernli]

Same as above; this utility is only called by Exp and ExpFrac that already check for an empty Pauli array.