Fix TDM "modes" validation to match XQC API

[lneuhaus]

Context:
Device specifications for TDM programs that are exposed by Xanadu's device API used to include a field modes with data like

{
    "concurrent": 2,
    "spatial": 1,
    "max": {"temporal": 100},
}

This format was recently changed into (ex generis):

{
    "concurrent": 2,
    "spatial": 1,
    "temporal_max": 100
}

Description of the Change:
This PR brings Strawberry Fields up to date with the new format.

Benefits:
TDM programs can run on Xanadu hardware.

Possible Drawbacks:
None.

Related GitHub Issues:
None.

[codecov]

Codecov Report

Merging #566 (d3fb71e) into master (25ab8d4) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #566   +/-   ##
=======================================
  Coverage   98.36%   98.36%           
=======================================
  Files          76       76           
  Lines        8332     8332           
=======================================
  Hits         8196     8196           
  Misses        136      136           

Impacted Files	Coverage Δ
strawberryfields/tdm/tdmprogram.py	95.72% <100.00%> (ø)

---

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 25ab8d4...d3fb71e. Read the comment docs.

[lneuhaus]

Is the weird behavior of codecov expected? I just changed a few lines, no way to have decreased coverage by 40% ;)

[heltluke]

I guess test_tdmprogram.py needs to be updated too, but otherwise looks good Leo!

[antalszava]

Looks good overall! 🙂 Thank you! Indeed, as Luke mentioned, probably this line needs an update.

[josh146]

Hey @lneuhaus, since SF now uses API versioning, my understanding is this shouldn't be changed unless the pinned API version is also increased as needed.

Unless somehow the current API version that SF targets was changed?

But maybe someone from platform can clarify this (@jswinarton?)

Is the weird behavior of codecov expected? I just changed a few lines, no way to have decreased coverage by 40% ;)

I think this is because the

Tests / core-tests (0, frontend or apps or api or gaussian)

CI is failing, so it is not updating its coverage info to Codecov. And this CI check tests the majority of the code base :)

[lneuhaus]

since SF now uses API versioning, my understanding is this shouldn't be changed unless the pinned API version is also increased as needed.
Unless somehow the current API version that SF targets was changed?
But maybe someone from platform can clarify this (@jswinarton?)

What I know for sure:

• all API versions >= 0.3.0 serve un-nested dicts, so should be in agreement with this patch.
• API version 0.1.0 does not have a device spec endpoint, so is irrelevant.
• API version 0.2.0 (the one SF currently requests (see here) specifies a nested dictionary with structure {"max": {"key": value}}. It's not entirely clear to me what keys the specification allows here apart from "max" in the top-level dict, but the only allowed value types here should be dictionaries (i.e. also not integers). From that perspective the current SF implementation is not aligned with the spec for API version 0.2.0 already (since it expects {"concurrent": 2} for example), so I believe we are in a gray zone here anyways.

Either we admit that the API 0.2.0 spec is wrong and expand it

• from modes: Union[int, Dict[str, Dict[str, int]]]
• onto modes: Union[int, Dict[str, Union[int, Dict[str, int]]]]

to be consistent with what's being returned by the API anyways, or we take the API 0.2.0 as the ultimate source of truth, in which case I could modify this PR to take that into account.

Definitely curious to get @jswinarton, @gtr8, or @Mandrenkov 's input on this.

[Mandrenkov]

• all API versions >= 0.3.0 serve un-nested dicts, so should be in agreement with this patch.

True, but I think this statement is weaker than you think. Here is how the 0.3.0 endpoint is currently implemented:

• If modes is not a dictionary (e.g., an integer), return it without modification.
• If modes is a dictionary and the target starts with simulon, flatten the modes into a Dict[str, Any] and return it.
• If modes is a dictionary and the target does not start with simulon, return an empty dictionary

If you try and request the device specifications for a TD2 device using the 0.3.0 endpoint, you will get an empty dictionary.

• API version 0.2.0 (the one SF currently requests (see here) specifies a nested dictionary with structure {"max": {"key": value}}. It's not entirely clear to me what keys the specification allows here apart from "max" in the top-level dict, but the only allowed value types here should be dictionaries (i.e. also not integers). From that perspective the current SF implementation is not aligned with the spec for API version 0.2.0 already (since it expects {"concurrent": 2} for example), so I believe we are in a gray zone here anyways.

The platform 0.2.0 API specification does indeed state that modes is either an integer or a dictionary of the form {"max": {"key": value}}; however, the 0.2.0 implementation does not perform any validation in this regard. So, if you request the TD2 device specifications using the 0.2.0 endpoint, you will receive

"modes": {
  "equal": {
    "concurrent": 2,
    "spatial": 1
  },
  "max": {
    "temporal": 100
  }
}

Note the "equal" key above; this is exactly what the TD2 device specifications return today.

Either we admit that the API 0.2.0 spec is wrong and expand it

• from modes: Union[int, Dict[str, Dict[str, int]]]
• onto modes: Union[int, Dict[str, Union[int, Dict[str, int]]]]

to be consistent with what's being returned by the API anyways, or we take the API 0.2.0 as the ultimate source of truth, in which case I could modify this PR to take that into account.

I think performing either change is not great for semantic versioning so we should do the one that leaves the API in the most desirable state. That is, we either

1. Fix the 0.2.0 API implementation to transform the TD2 device spec into a Dict[str, int], or
2. Update the 0.2.0 API specification to allow for a more flexible device spec.

I'm not sure which of these options is more convenient for Strawberry Fields.

[lneuhaus]

@Mandrenkov and I have run a test with this branch - it is now in line with how the platform presents the modes data, from API version 0.2.0 onwards. So

Fix the 0.2.0 API implementation to transform the TD2 device spec into a Dict[str, int]

is what both the platform and this PR went with, so I suggest to simply merge this PR and move on.

[Mandrenkov]

Looks good to me.

As @lneuhaus mentioned, the discrepancy can be fixed by updating the TD2 hardware certificate version; however, it will be a week or so before we deploy support for the new certificate version on the platform side. I think it would sense to hold off merging this PR until then (to minimize the TD2 downtime).

[lneuhaus]

to minimize the TD2 downtime

TD2 is effectively down until this fix is applied since a platform upgrade around January (introduction of device API) - therefore I suggest we merge this now in order to be sure to integrate this behavior into the next Strawberry Fields release.

[Mandrenkov]

Okay, sounds good!