Sequence sampler #345

lvignoli · 2022-03-14T18:19:10Z

Close #342.

A working but unfinished Sequence sampler. The tests fail on purpose: I am still thinking about the best way to test this. Also, not all noise sources have been considered.

A new module called sampler exposes the sampler.sample() function, which is given a sequence in argument and returns a nested dict of Global and Local samples per basis. The construction of this dict mimics the one in the simulation module.

Arguments given to sampler.sample() allow for modulation and noise models relevant at the sampling level.

Some comment on the internals:

Samples and QubitSamples are two data class to holds and pass the samples around before pasting them to the dict
A global channel can "decay" to a local one because of random-per-qubit noises or an SLM.
Global channel, local channel or a global decayed channel are handled a bit differently by `sampler.sample()
Noises are modeled by functions, see the NoiseModel alias ; we expose generators from the sampler.noises module to construct the desired noise function

plain dataframe of samples extract samples in the nested dict form A note: basis that are unused by declared channels are arrays full of zeros, while it is simply not initilized in the dict in the simulation module. add test to compare with extraction from simulation module Add various docstrings type annotations sample per qubit, test passes add support for LocalNoises add seed to the amplitude_noise generator fix the doppler noise to use a normal distribution Write test for doppler noise. It fails. It fails probably because of the different seeds in the random generators ? refactor a bit Rename as module sampler Refactor by distinguishing the local and global extraction Remove unused import Refactor the noises to their own modules Add modulation, but no implementation Revert "Refactor the noises to their own modules" This reverts commit 37093330c57a9152604f239e79ec6940a213b432. Use strategies for _TimeSlot extraction Remove functions made useless by refactor to strategy pattern Add doctrings and make some functions private Extend test to local channels Remove dead code Add modulation Refactor a bit: docstrings, order of functions, etc. Fix _TimeSlot grouping by handling delays correctly Tidy the comments refactor the extraction strategies Docstring

Deprecate temporarily the amplitude_noise as it misses the point It urges for a correct implementation Refactor sample writing Allow local decay with on SLM Add support for noises on global channel Add TODO for global modulation Fix usage of noises.apply() Fix modulation and apply SLM on local channels as well Fix imports

Also fix imports for mypy compliance

HGSilveri · 2022-03-15T09:15:35Z

One first comment from just looking at the file structure: it's important that we keep simulation-specific features contained in the pulser.simulation module so that we can later separate it from the rest of the library easily.

Adds a post_init check for same length of quantities arrays Add documentation, testing and refactoring Refactor the dict construction with modulation Simple comment to clarify the design of sampler.sample() Remove unnecessary value in enum _GroupType Add a diagram for _TimeSlot grouping Fix embedded NoiseModel by using copies Update docstrings with args Expose the sample() function from the __init__ Test the amplitude noise Remove unused commented import Add usage note on doppler noise Add a docstring for the amplitude noise Remove empty line Create a better test for doppler noise Small refactor Mark the doppler test as expected to fail Refactor the control flow of sampler.sample() Refactor the dict exporting Refactor Fix typo Add a docstring Refactor the TimeSlot grouping and strategy

Global and local channels are sampled identically. The export to a dict form like the simulation one handles the distinctions. It introduces an additional dict keeping track of the addressing of each channel: Global, Decayed, Local. Trim dead code, refactor, defensive coding Write global channel from QubitSamples Rename modulation function

Table based tests Add a modulation test I don't like it. It is testing the sampling for sure, but still I would like to test against a known output of the modulation. Remove unnecessary if branch functools.reduce got us covered already Extend to digital Fix mypy error because of weird scope of variables Test sequence in XY and fix a typo in the relevant code Test for corner cases and omit defensive coding with pragma no cover

lvignoli · 2022-03-22T11:01:54Z

The latest commits make the sampler a working feature.

QubitSamples is the only data structure of the module. It holds samples at the ns scale for the amplitude, phase and determinant attached to one qubit. An instance accounts for all the _TimeSlot of one channel or only some of them (or all the _TimeSlot of all channels if even necessary).
modulation is working
internals have been refactored and simplified, so that processing of channels is more streamlined regardless their type (global or local).
there are tests with total coverage

A comment on the design choice: for now I think a function is enough.
It is easy to test (no object to instantiate) and the usage is very simple

sampler.sample(seq)
# just like
json.dumps(obj)

Though, I am not satisfied with the way noises are specified. It will be reworked with along with the other features I want to add. So it may evolve towards a more complex implementation.

lvignoli · 2022-03-22T11:13:58Z

It should close linked issue #342 and make PR #337 obsolete.

HGSilveri

Here is a non-exhaustive review of what you've done so far (I'm still missing most of the tests).

I would say that overall, there is a tendency to abuse helper functions in a way that makes the code harder to read sometimes. I left a couple comments on that, but I feel that overall the code could use a going over with that in mind.

Other than that, it seems to be in a good shape already!

[Note: Actually, GitHub seems to be crashing so I'll leave these comments for now and follow up with the ones I've lost]

pulser/sampler/noises.py

pulser/sampler/sampler.py

pulser/tests/test_sequence_sampler.py

Sequence._slm_mask_time is not a list of times, but a pair with a start and end time of the SLM masking.

For global channels, we hold the same data in N copies, N the number of qubits of the register. It scales poorly with the size of the register, but it remains manageable for current size of registers. If needed, we should patch this, at the expense of a more complex control flow in the sample() function at least.

pulser/tests/test_sequence_sampler.py

pulser/sampler/sampler.py

pulser/tests/test_sequence_sampler.py

lvignoli · 2022-03-28T12:28:35Z

Apart from the doppler noise effect discussion—which fails on purpose now—it looks nice to me.

HGSilveri

Looks good to me too! Let's go with the current implementation of Doppler noise, it doesn't make sense to delay this PR because of it.

lvignoli · 2022-03-28T16:54:29Z

🎉

Main changes: - 3921795 Remove string variables (#365) - b23ff22 Splitting `pulser` into `pulser-core` and `pulser-simulation`. (#362) - 4f684f3 Address qubits with indices (#356) - f2ee5e2 Make evaluation times uniform including final time (#330) - c34dffb Changes for compatibility with upcoming export format (#353) - 3ac1f66 Sequence sampler (#345)

lvignoli added 5 commits March 14, 2022 19:07

Refactor sampler in independent module

94fbdb2

Refactor the noises module

5b5f95b

Also fix imports for mypy compliance

Improve docstrings and small refactoring

7459af5

lvignoli added the enhancement New feature or request label Mar 14, 2022

lvignoli added this to the v0.6 Release milestone Mar 14, 2022

lvignoli self-assigned this Mar 14, 2022

lvignoli mentioned this pull request Mar 14, 2022

Sequence sampler component #342

Closed

Small typos in comments

c0ff5d7

lvignoli added 7 commits March 16, 2022 14:35

Fix the decay of global channels to local ones

79fcd26

Fix the modulation feature

d983031

Simplify the sampling by removing the Samples dataclass

1220cbd

Fix a docstring

949cfd0

lvignoli requested a review from HGSilveri March 22, 2022 11:12

lvignoli mentioned this pull request Mar 22, 2022

Sequence sampler feature extension #349

Closed

4 tasks

HGSilveri reviewed Mar 22, 2022

View reviewed changes

lvignoli added 7 commits March 23, 2022 10:45

Fix docstrings formatting and content

b2f0e3e

Remove unnecessary nonzero check in amplitude noise

8d09f12

Fix the noise seed default value: defaults to None

9f35042

Improve docstrings of helper functions in noise module

03e8558

Apply noises before the SLM masking

19af68d

Rename misnamed variables

14f9c8b

Change the scope of _key_func() and remove the _GroupType class

41d175d

lvignoli mentioned this pull request Mar 25, 2022

Update runs and samples per run. #351

Merged

lvignoli added 9 commits March 25, 2022 15:39

Fix SLM masking: consider only the seq._mask_times

85cbdbf

Sequence._slm_mask_time is not a list of times, but a pair with a start and end time of the SLM masking.

Change for defaultdict in new_qdict

a88c31d

Simply testing of sequence

0d752fd

Refactor the testing of sequence with a helper function

77d97e0

Fix the testing of blackman modulation

fe65aa4

Move noise-related tests to an independent testing files

1f05e03

Fix the unneeded # pragma: no cover in _sample_slots

2c8c07a

Add a Failing test for the SLM, for discussion

9697a2d

lvignoli commented Mar 25, 2022

View reviewed changes

pulser/tests/test_sequence_sampler.py Show resolved Hide resolved

HGSilveri reviewed Mar 25, 2022

View reviewed changes

lvignoli added 7 commits March 28, 2022 10:15

Move the comment on performance

198c094

Make the SLM detectino more idiomatic

373e10f

Simplify the assertions for sequence in XY

0720c50

Remove superfluous print statements

eeeba30

Test SLM sampling alongside the check against simulation

0201e14

Refactor using a helper for nested dicts

aefbc9e

Rearrange the test file

35d4b75

HGSilveri approved these changes Mar 28, 2022

View reviewed changes

Merge branch 'develop' into WIP/sampling

12c9344

HGSilveri merged commit 3ac1f66 into pasqal-io:develop Mar 28, 2022

This was referenced Mar 28, 2022

WIP: add modulation to the simuation module #337

Closed

Add option to simulate a Sequence with the modulated output #315

Closed

HGSilveri mentioned this pull request May 13, 2022

Release v0.6.0 #372

Merged

lvignoli deleted the WIP/sampling branch May 19, 2022 15:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sequence sampler #345

Sequence sampler #345

lvignoli commented Mar 14, 2022

HGSilveri commented Mar 15, 2022

lvignoli commented Mar 22, 2022

lvignoli commented Mar 22, 2022

HGSilveri left a comment

lvignoli commented Mar 28, 2022

HGSilveri left a comment

lvignoli commented Mar 28, 2022

Sequence sampler #345

Sequence sampler #345

Conversation

lvignoli commented Mar 14, 2022

HGSilveri commented Mar 15, 2022

lvignoli commented Mar 22, 2022

lvignoli commented Mar 22, 2022

HGSilveri left a comment

Choose a reason for hiding this comment

lvignoli commented Mar 28, 2022

HGSilveri left a comment

Choose a reason for hiding this comment

lvignoli commented Mar 28, 2022