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

Refactor Microsoft.Quantum.Preparation for consistency and to use style guide. #211

Closed
9 tasks
cgranade opened this issue Jan 23, 2020 · 5 comments
Closed
9 tasks
Assignees
Labels
Kind-Bug Something isn't working Resolution-Done Issue closed because work item completed.

Comments

@cgranade
Copy link
Contributor

cgranade commented Jan 23, 2020

Describe the bug
The current Microsoft.Quantum.Preparation namespace, which provides functions and operations for preparing quantum registers in a variety of different states, currently suffers from a number of different usability and consistency issues:

  • The name of the PrepareQubit operation does not accurately reflect what it does (namely, that it prepares the positive eigenstate of a single-qubit Pauli operator). (originally reported by @tcNickolas)
  • API documentation comments are inaccurate and lack examples (the latter as noted in Provide more examples in API documentation comments #113)
  • API documentation comments use mathematical notation in summaries, breaking hover information in LSP clients (e.g.: VS Code and VS 2019).
  • API documentation comments refer to "returning unitaries" rather than returning operations.
  • Private callables indicated by trailing rather than leading underscores; this should go away with adapting to use Access modifiers for callables and types qsharp-compiler#259, but it would be good to be consistent in the meantime.
  • The StatePreparationComplexCoefficients and StatePreparationPositiveCoefficients functions are identical up to the type of their expected inputs, rather than using type suffixes.
  • StatePreparationPositiveCoefficients can be readily generalized to allow negative coefficients as well.
  • The output type of QuantumROM is confusing, and can be clarified by defining a new UDT; this does present additional API documentation concerns, however, given Allow documenting named items of UDTs qsharp-compiler#113.
  • The name QuantumROM preferences the implementation above the thing being implemented, and should be renamed accordingly.
@cgranade cgranade added the Kind-Bug Something isn't working label Jan 23, 2020
@cgranade cgranade self-assigned this Jan 23, 2020
@cgranade
Copy link
Contributor Author

Possible action items for resolving type specializations of arbitrary state preparation:

  • Deprecate StatePreparationComplexCoefficients, as this function simply partially applies PrepareArbitraryState, and is currently only used to in cases such as:
(StatePreparationComplexCoefficients(coefficientsNewComplexPolar))(LittleEndian(auxillary));
  • Deprecate StatePreparationPositiveCoefficients in favor of a new PrepareArbitraryStateD and rename PrepareArbitraryState to PrepareArbitraryStateCP.

@tcNickolas
Copy link
Member

Thank you for this systematic approach!

One more thing in this library I've seen confuse people is that the operations only describe what happens if the input state is 0...0. It's unclear what happens if the input state is different - does the operation fail altogether, does it reset all qubits to 0 before preparing the state, does it apply some kind of transformation that ends up not preparing this state? It would be very helpful to clarify this.

@cgranade
Copy link
Contributor Author

@tcNickolas: No worries, happy to continue the work done with the amplitude amplification namespace with this namespace as well; thanks for highlighting preparation as an area of interest so that I can prioritize accordingly.

As for the point about states other than |00⋯0⟩, my approach thus far has been that the meaning of the "Prepare" verb is only well-defined for a particular initial state (|00⋯0⟩ for bare registers, |0⟩ for little-endian registers, and so forth), such that acting on other input states is undefined behavior. That is, "Prepare" operations generally act more like partial isometries than unitaries. Where additional specific behavior can be reasonably defined (e.g.: preparing a Bell state with an input of |10⟩ makes perfect sense, and should prepare (|00⟩ − |11⟩) √2), that should be called out in API documentation comments as a /// # Remarks section.

I think it's a bit hard to enforce that input registers are passed in the right input states, as that could introduce a significant number of additional Reset or ResetAll operations, and would make it harder to use Adjoint of a preparation to represent something that coherently unprepares a given state. Similarly, resetting would make it hard to use controlled preparations common in distinguishability tests (e.g.: using the Hadamard test to tell if the states prepared by two different operations are approximately the same or not; for the SWAP test you wouldn't need controllability). It may similarly be suboptimal to use assertions to enforce on a simulator, but I think that would be the best alternative should we want to avoid undefined behavior.

It may be helpful to clarify that, then, as part of the namespace description, and in the forthcoming API principles.

@cgranade
Copy link
Contributor Author

Apologies @KittyYeungQ, I forgot to tag you in for awareness on the point about documenting the meaning of the verb "Prepare."

@cgranade
Copy link
Contributor Author

Closed with #344.

@cgranade cgranade added the Resolution-Done Issue closed because work item completed. label Nov 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Kind-Bug Something isn't working Resolution-Done Issue closed because work item completed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants