quest-kit / quest Goto Github PK

• The multiqubit object shouldn't be accessed directly as the properties on that object aren't defined in the API. In particular, number of amplitudes means number of amplitudes per MPI rank in the distributed case, which happens to be total number of amplitudes in the whole system in the local case. It will be safer to access the number of amplitudes from an API function

Even in quad precision, brings trace from 1 to 0.95.

This shouldn't be too hard to make contiguous, making it much faster due by reducing caches

QuESTAsserts should be in e.g. QuEST_ops, rather in a hardware backend (e.g. QuEST_gpu) which leads to code duplication.

It would be great to run some C formatter on the source code, just to make it a bit more uniform, just as in places there is code like

void example(){

}

then something like

void example() {

}

or

void example()
{

}

It would just make the source a little more readable, but thats just my preference!
I will do a pull request, but please fell free to ignore if needed, if you think it's not necessary.

Currently, these are

• CPU MPI densmatr_initPureState
• CPU MPI densmatr_collapseToKnownProbOutcome
• CPU local densmatr_collapseToKnownProbOutcome

the density analogs of the debugging functions (if necessary)

• initStateDebug
• initStateFromSingleFile
• initStateOfSingleQubit
• reportStateToScreen

• Add a function to initialise the state vector of probability amplitudes to be uniformly (?) random

All other density matrix functions are supported on all platforms.

Also shorten QASM REAL strings

These lines:

sizeHalfBlock = 1LL << targetQubit;  
sizeBlock     = 2LL * sizeHalfBlock; 

and

        thisBlock   = thisTask / sizeHalfBlock;
        indexUp     = thisBlock*sizeBlock + thisTask%sizeHalfBlock;
        indexLo     = indexUp + sizeHalfBlock;

are duplicated in almost every *Local function in QuEST_cpu.c.
Should they instead be fetched by something like

getStateVecIndices(targetQubit, thisTask, &indexUp, &indexLo)

• to add two probability-weighted density matrices (or one pure)
• explicit trace function (currently calcTotalProbability)
• trace of density matrix squared
• fidelity

Only some runtimes - I've probably goofed up some pointer

• add a function which takes the inner product of the state vectors of two multiqubit objects that contains the same number of qubits

• While internal functions are documented using doxygen, it would be useful to have a higher level description of how the code works, particularly how MPI/non-mpi functionality is handled, for developers

Since this is the number of qubits suggested by the dimensions of the state-vector, which is twice too many when the register is storing a density matrix (and should instead refer to numDensityQubits).

Consider renaming

• numDensityQubits -> numQubitsRepresented
• numQubits -> numQubitsInStateVec

Still seems hacky and gross ¯\(ツ)/¯

• Is there a way to unit test this?
• Do the current methods to get time, pid and hostname in QuESTSeedRandomDefault fail on any architectures or OS?

Possible now that gates have a central, hardware-agnostic place in QuEST_ops.h

tGate, sGate and sigmaZ can then be calculated error-free

phaseFlip should then wrap phaseShift in QuEST_common.c

• Update the MultiQubit object or add a new MultiQubitWithErrors object that contains information about errors that should be introduced during each operation on the qubit
• Could be a small error with every operation or a probability of a larger error with each operation

This involves scaling the input (so that, e.g. maximum dephasing occurs when an argument of 0.5 [probability of X] is passed) and adding new validation to restrict probs to be below their max-mixing values.

• need to host the API from each release somewhere, as the current doxygen/github pages system will only show the most recent version

Lets the user supply amplitudes to set in the (possibly distributed) state-vector.
Something like
initStateByAmps(reg, reals, imags, numAmps, startInd)
would allow writing chunks at a time, needed when distributed.

What is currently

combineDensityMatrices(0.3, targQureg, 0.7, otherQureg);

should become

addDensityMatrix(targQureg, 0.7, otherQureg);

or something

Make the method robust against this

• getRealAmpEl
• getImagAmpEl
• getProbEl

Also, should exist getAmplitude which returns Complex for state-vectors.

• initStatePlus -> initPlusState
• initStateZero -> initZeroState
  brings them more in-line with initClassicalState and initPureState.
  Is init the best prefix?

Since we allow user source code to be either C or C++ for GPU QuEST.

• controlledHadamard might be equally expensive as controlledUnitary
• sGate and tGate are easy - change controlledPhaseShift to that ByTerm (like for single qubit)

• regular gates (very tedious though - particular attention to measurements)
• calcFidelity
• calcPurity
• calcTotalProbability
• oneQubitDephase
• twoQubitDephase
• oneQubitDepolarise
• twoQubitDepolarise
• combineDensityMatrices
• getDensityAmplitude

They currently have no doc comment in QuEST.h

It's especially important to unit test with many different numbers of machines, since some code has different logic based on whether contiguous amplitude sets fit entirely into a chunk.

• Update internal documentation, particularly looking at consistency between documentation on distributed/local versions of particular functions

Need rigorous testing for density matrices

Should this be a global constant if its the safe for all kernels?

controlledSigmaX is implemented with controlledNot, and controlledSigmaZ is implemented with controlledPhaseFlip

Test 7 Hadamard fails when QuEST_PREC=1 (4 bytes)

• can be used for checkpointing

E.g. initQuESTEnv

• While the state vector of probability amplitudes wouldn't be updated in usual use, we have seen cases where the user extends the code by updating this vector directly. This code might break if the program was switched from running locally to running across multiple nodes, so it is safer to offer setters which can handle distributed mode in the API

• Current makefile system isn't immediately portable to windows
• Current makefile system is potentially harder to configure than a cmake system
• Add option to compile QuEST as a library

Should summing probabilities be done by reduction?
Otherwise, should mixed_findProbabilityOfOutcome also just iterate with Kahan summation?

Why should we make the user supply a reference to the struct in the stack, when the create... and init... funcitons could just as easily malloc space for the struct in the heap and return that?
We already require explicit freeing functions to free their dynamic attribs.

This would mean the creation of these structs is in one line...

QubitRegister* qureg = createQubitRegister(5);

as opposed to

QubitRegister qureg;
createQubitRegister(&qureg, 5);

It also means the users naturally have a handle to a pointer rather than the struct - all gate functions could then consistently accept a pointer (to be negligably more efficient by not having to copy the struct fields).
E.g. we currently do

QubitRegister qureg;
createQubitRegister(&qureg, 5);
initStateZero(qureg);

but could instead do

QubitRegister* qureg = createQubitRegister(5);
initStateZero(qureg);

where initStateZero now accepts a pointer.