add seed list feature

[cqc-melf]

Fixes the problem discussed in CQCL/pytket-qulacs#44

[cqc-alec]

Why is it necessary to provide a list of seeds, instead of just one seed which gets auto-incremented on each submission? This would give reproducibility and (assuming a half-decent PRNG) remove any bias. Is there a use case I'm missing?

[cqc-melf]

My understanding is that is this only taking affect when doing batch submissions. I was thinking of the case that you want to rerun your experiments for only a subset of your initial input. When doing auto incremented seeds this would not work.

I think this is an unlikely use case, and quite confusing to use. If I understand the code, batching here is done by grouping together circuits with the same number of shots; so you would need one seed per distinct value in your n_shots list, and when running a subset you would be very restricted in which subsets you could run (while still having reproducibility). All this would need to be explained in the docs. I think I'd rather keep things simple instead of adding a feature for which there's no clear need.

[cqc-alec]

See #197 (comment) . (Sorry, looks like I edited your comment instead of adding a new one.)

[cqc-melf]

My understanding is that is this only taking affect when doing batch submissions. I was thinking of the case that you want to rerun your experiments for only a subset of your initial input. When doing auto incremented seeds this would not work.

I think this is an unlikely use case, and quite confusing to use. If I understand the code, batching here is done by grouping together circuits with the same number of shots; so you would need one seed per distinct value in your n_shots list, and when running a subset you would be very restricted in which subsets you could run (while still having reproducibility). All this would need to be explained in the docs. I think I'd rather keep things simple instead of adding a feature for which there's no clear need.

The list of shots contains one entry per circuit and in the same way I am expecting one entry in the seeds list per circuit.

If all the seeds are identical (or the user just gives one value like before) we can do the submissions of circuits batches using the qiskit-batch interface (this still works in the same way as before) (see https://github.com/CQCL/pytket-qiskit/pull/197/files#diff-8ccf4df5e4b2a5aa355a1a8a3086114f67945bdab047dc9f75ed25ed7dfd0850R78 )

If there are different seeds given, doing the creation of the qiskit-batches needs to be done differently: each batch now only contains exactly one circuit and the corresponding seed and shot number. (see https://github.com/CQCL/pytket-qiskit/pull/197/files#diff-8ccf4df5e4b2a5aa355a1a8a3086114f67945bdab047dc9f75ed25ed7dfd0850R92 )

If we want to do something like autoincrementing the seeds, we would need to break the qiskit-batch creation anyway. I have not found a way to have different seeds for the list of circuits in a qiskit batch.

So I think the user is free to choose any subset they would like and this would still work. (But if you think this is not a use case that is going to happen, I am happy to change to auto incrementing the seed)

[cqc-alec]

The list of shots contains one entry per circuit and in the same way I am expecting one entry in the seeds list per circuit.

Ah, this is indeed the case, but only because you have changed the logic of _batch_circuits(). It used to group circuits having the same number of shots together, but now it just puts one circuit in each batch (unless all the seeds are the same).

I assume this behaviour was in order to make most efficient use of the IBMQ interface so I think we should keep it.

[cqc-melf]

The list of shots contains one entry per circuit and in the same way I am expecting one entry in the seeds list per circuit.

Ah, this is indeed the case, but only because you have changed the logic of _batch_circuits(). It used to group circuits having the same number of shots together, but now it just puts one circuit in each batch (unless all the seeds are the same).

I assume this behaviour was in order to make most efficient use of the IBMQ interface so I think we should keep it.

From my understanding the IBMQ interface only allows to set one seed for each qiskit-batch, so we would have to do equal seeds per qiskit batch. That would show the same issue as we had before, do we want to do that and would not be compatible with auto incrementing seeds as well?

[cqc-alec]

From my understanding the IBMQ interface only allows to set one seed for each qiskit-batch, so we would have to do equal seeds per qiskit batch. That would show the same issue as we had before, do we want to do that and would not be compatible with auto incrementing seeds as well?

What is the issue with one seed per qiskit batch? I would assume that the seed is used once to seed a global PRNG for the whole batch, not one PRNG per circuit.

[cqc-alec]

If a user wants to have reproducibility at the sub-batch level, I think they need to design that into their workflow from the beginning: each call to process_circuits() should be reproducible, and therefore only needs a single seed.

[cqc-melf]

I have removed the list feature and added a parameter for auto inc in 080f453

But I am not fully sure this is helpful like this, I would assume that this might still encourage people to not use batch submission when they want to reproduce results reliably.

[cqc-alec]

I think this should be automatic (definitely the default behaviour, but I don't see a reasonable use case for not incrementing the seed, which introduces biases in the results, so no need to add a new kwarg).

[cqc-melf]

I did not wanted to do this by default because that would stop old runs from being reproducible, if that is not a concern I am happy to remove the parameter and always do the auto inc. The use case for the parameter I see is being compatible with reproducing old runs.

[cqc-alec]

At least they will still be reproducible if the same version numbers (of pytket-qiskit, qiskit, etc) are used.

[cqc-alec]

I have removed the list feature and added a parameter for auto inc in 080f453

But I am not fully sure this is helpful like this, I would assume that this might still encourage people to not use batch submission when they want to reproduce results reliably.

If incrementing the seed is automatic then every call to process_circuits() should be reproducible (if the same seed is used).

[cqc-alec]

Update description in changelog.

[cqc-melf]

Done in 85489ef

[cqc-alec]

Remove.

[cqc-melf]

Done in 85489ef

[cqc-alec]

Suggested change

	if kwargs.get("seed_auto_increase") and type(seed) is int:
	seed += 1
	seed += 1

[cqc-melf]

Done in 85489ef

[cqc-alec]

Remove.

[cqc-melf]

Done in 85489ef

[cqc-alec]

Suggested change

	if kwargs.get("seed_auto_increase") and type(seed) is int:
	seed += 1
	seed += 1

[cqc-melf]

Done in 85489ef

[cqc-melf]

I needed to keep the type check for mypy. (and to make sure I am not incrementing when the seed is set to none)