Time domain program class

[nquesada]

Context: Implements the time domain program class Description of the Change:

Benefits:

Possible Drawbacks:

Related GitHub Issues:

[codecov[bot]]

Codecov Report

Merging #440 into master will decrease coverage by 1.01%. The diff coverage is 99.39%.

@@            Coverage Diff             @@
##           master     #440      +/-   ##
==========================================
- Coverage   97.84%   96.83%   -1.02%     
==========================================
  Files          58       70      +12     
  Lines        6784     7077     +293     
==========================================
+ Hits         6638     6853     +215     
- Misses        146      224      +78     

Impacted Files	Coverage Δ
strawberryfields/apps/clique.py	100.00% <ø> (ø)
strawberryfields/apps/plot.py	100.00% <ø> (ø)
strawberryfields/apps/train/embed.py	100.00% <ø> (ø)
...awberryfields/backends/gaussianbackend/__init__.py	100.00% <ø> (ø)
strawberryfields/backends/gaussianbackend/ops.py	100.00% <ø> (+2.70%)	:arrow_up:
strawberryfields/backends/shared_ops.py	96.11% <ø> (-0.98%)	:arrow_down:
strawberryfields/engine.py	95.45% <ø> (+0.55%)	:arrow_up:
strawberryfields/io.py	97.70% <ø> (-1.13%)	:arrow_down:
strawberryfields/ops.py	98.84% <ø> (-0.18%)	:arrow_down:
strawberryfields/parameters.py	99.06% <ø> (+0.03%)	:arrow_up:
... and 74 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 6c12c3f...39dad08. Read the comment docs.

[nquesada]

@josh146 There are a couple of things that pylint is complaining about and that I am not sure are intentional. Wondering if you could have a look? If they are intentional you can let me know and I can add the relevant pylint disables

[nquesada]

Finally, just to confirm: Do we want eliminate any and all reference to shift=after? Two options here:

1. A hard removal where we can simply remove the shift option altogether and it is always assumed that it amount to shift=end
2. Leave the keyword shift in there with the default value? I the user uses any other value for shift we raise a warning. If in the future we decide to add more options for shift it will be simpler to add.

What do @josh146 and @fab1an-q think?

[fab1an-q]

We could give the users the option to pass an integer as shift parameter, defining the step size of the qumode shift in each time bin. Might be interesting for some to experiment with it.

However, I think we should change the string "end". The name was useful to distinguish from shift="after", but we decided to remove this option (I believe?). So maybe something like shift="auto" (as compared to a user-controlled shift) makes more sense?

[fab1an-q]

Just realized that the option for integers as shift parameters is currently implemented in the TDMProgram:

elif isinstance(self.shift, int):
    q = shift_by(q, self.shift)  # shift at end of each time bin

I think we could leave it this way. But if our automated shift-handling works as fine as we hope, I acknowledge that only few people will not want to use it. So I am also ok with removing shift entirely from the arguments of the context manager.

[josh146]

However, I think we should change the string "end"

:+1:

I think we could leave it this way. But if our automated shift-handling works as fine as we hope, I acknowledge that only few people will not want to use it. So I am also ok with removing shift entirely from the arguments of the context manager.

If it is less effort to leave it in/as-is, I'm happy to leave it as is!

[nquesada]

Alright @thisac , @fab1an-q and @josh146 . I think we are close to the end. I have lightly reformatted the great functions @fab1an-q wrote into proper tests by adding assert statements and calculating the theoretical values expected.

[josh146]

Sorry everyone, black seems to have changed its mind over what formatting it prefers - a whole heap of unrelated files have been added to this PR! Please ignore them :)

[josh146]

I expected ops.MeasureHomodyne(p[1]) here...?

@fab1an-q oops, yes, fixed. Good catch!

[josh146]

@nquesada it looks fine to me?