[cirqflow] KeyValueExecutableSpec should provide a to_dict method / override __getitem__

[tanujkhattar]

Is your feature request related to a use case or problem? Please describe.
cg.KeyValueExecutableSpec provides a nice from_dict() method to convert a dict into a Tuple[Tuple[str, Any], ...] which is hashable. This is useful when constructing the executable spec. However, using the executable spec during analysis of the results forces one to use the stored tuples, which is cumbersome.

Describe the solution you'd like
The class should provide a similar to_dict method which can convert the stored key_value_pairs to a dictionary and return -- which are much easier to work with. Though the method would be a simple return dict(self.key_value_pairs), there might be some value in explicitly having it on the class. We can also consider providing a custom __getitem__ method.

What is the urgency from your perspective for this issue? Is it blocking important work?
P1 - I need this no later than the next release (end of quarter)

cc @mpharrigan

[mpharrigan]

sounds like a good idea to me

[viathor]

I agree with @mpharrigan. Added triage/discuss to decide which options to go with.

[tanujkhattar]

Discussed in cirq sync: We should have a to_dict method. Probably also investigate whether the class needs to be immutable since cirq json serialization can handle dict to sorted tuples conversion.

Another heads up here is that dict -> tuple conversion should sort tuples to guarantee stability. In this class, we do not currently do the sorting. cc @mpharrigan

[mpharrigan]

dictionaries are ordered in modern python. it's not clear to me whether we should be sorting.

[tanujkhattar]

They are insertion ordered, not sorted based on keys. One counter intuitive example due to this is:

> d1 = dict.fromkeys(['a', 'b'])
> d2 = dict.fromkeys(['b', 'a'])
> assert d1 == d2
> k1 = cg.KeyValueExecutableSpec.from_dict(d1, executable_family='test')
> k2 = cg.KeyValueExecutableSpec.from_dict(d2, executable_family='test')
> assert k1 != k2  # k1 == k2 should ideally be True. 

[mpharrigan]

currently keyvaluespec has these semantics

KVSpec(stuff=(('a', 1), ('b', 2))) != KVSpec(stuff=(('b', 2), ('a', 1)))

so we should "fix" this as well.

[tonybruguier]

The proposed PR above is only for the new function. I have read about the key order issue, but it seems a tiny bit more dangerous, so I was planning to have a separate PR. My preference is weak though.