Parameterize TypeRow by element type #256

acl-cqc · 2023-07-10T13:53:32Z

There is a mass of churn here but in the end few major changes. There are two main benefits:

It makes assumptions clear, for example that (in this version, but largely inherited from main) predicates are classical-only.
The possibility of building a ClassicType::Container of a tuple including a Qubit is now statically ruled out
It avoids having to declare code to handle some cases that can never occur. For example, ConstTypeError::LinearTypeDisallowed can now be removed, and Signature::get for "other" ports no longer needs an error case

Note this does not follow #253; we could do #253 before or after this, and doing this first will involve significantly more churn, but also be more explicit and probably find more bugs...so I'm doing this first ;)

We retain that there are "kind of" two ways to declare the SimpleType tuple-of-two-ints:

SimpleType::Classic of ClassicType::Container of Container::Tuple of TypeRow of two ClassicType::i64()s
SimpleType::Qontainer of Container::Tuple of TypeRow of two SimpleType::Classic(ClassicType::i64())s

...the idea being that factory methods like new_predicate, new_tuple and new_sum will pick the correct one.

Drawbacks:

I had to change the serialization code (src/types/simple/serialize.rs) so that, rather than SerSimpleType containing TypeRows containing SimpleTypes, SerSimpleType now contains only (TypeRows of) SerSimpleTypes; flattening is thus recursively and once only rather than iteratively one stage at a time.
I've disabled some pyo3 for the time being. We might bring it back via wrapper SimpleRow and ClassicRow, say.

Secondly there is a new macro - type_row! builds TypeRow<SimpleType> only, there is now also classic_row!. However, both of these are used (almost) only in tests, so I have to wonder whether we need such control over efficient static allocation for test code. There is a possibility of renaming type_row! to simple_row! (another mass of churn!) but I'm holding off on that for now because we might rename SimpleType to Type or AnyType in which case type_row may be a reasonable name to keep. Renaming can follow in a later PR.

…he type

…ays Classical

… cargo build compiles

This reverts commit c48e4e7.

acl-cqc · 2023-07-10T13:55:12Z

src/types/simple.rs

- /// given the rows of each tuple
- pub fn predicate_variants_row(variant_rows: impl IntoIterator<Item = TypeRow>) -> Self {
- variant_rows
+ fn try_convert_elems<D: PrimType + TryFrom<T>>(self) -> Result<TypeRow<D>, D::Error> {


I'd like to just have

impl <F: PrimType, T: PrimType+TryFrom<F>> TryFrom<TypeRow<F>> for TypeRow<T>

but get a conflicting instance declaration...

acl-cqc · 2023-07-10T13:57:53Z

src/types/simple.rs

+ }
+
+ /// Converts the elements of this TypeRow into some other type that they can `.into()`
+ pub fn map_into<T2: PrimType + From<T>>(self) -> TypeRow<T2> {


Similarly, it'd be good for this just to be an Into/From impl, but again that seems to raise conflicting-instance problems

src/types/simple/serialize.rs

src/types/simple.rs

…erow_classic_predicate

…_typerow_classic_predicate

aborgna-q

Quite a bit of renaming noise, but LGTM

aborgna-q · 2023-07-24T15:40:08Z

src/types.rs

+pub use simple::{ClassicRow, ClassicType, Container, SimpleRow, SimpleType};

 use smol_str::SmolStr;

 use crate::hugr::{Direction, Port};
 use crate::utils::display_list;
 use crate::{resource::ResourceSet, type_row};

+use self::simple::{PrimType, TypeRow};
+


Export PrimType and TypeRow too?

So this is something I debate. Obviously Rust requires that these types have to be public, but I'd kinda like to encourage users to use the aliases. The case in which a user should be using TypeRow....almost certainly is in a function parameterized <T: PrimType> ...TypeRow<T>....

Which is not the common case, but it is a reasonable case ;-). So ok, exported (within types but no further, as nothing is exported out of types)

However @aborgna-q one thing we might do is make PrimType sealed (i.e. can only be implemented inside the crate)....?

src/types/simple.rs

aborgna-q · 2023-07-24T15:55:10Z

src/macros.rs

-/// let repeated_row: TypeRow = type_row![B; 2];
-/// assert_eq!(repeated_row, static_row);
+/// let repeated_row: SimpleRow = type_row![B; 3];
+/// assert_eq!(repeated_row, sig.output);
 /// ```
 #[allow(unused_macros)]
 #[macro_export]
 macro_rules! type_row {


I agree with the renaming... in a separate PR

aborgna-q · 2023-07-24T16:01:23Z

I have to wonder whether we need such control over efficient static allocation for test code

It's mostly useful when dealing with Signatures of leaf ops. OpType::signature on something like a CNOT does not need to allocate at all, so checking things on flat dataflow regions needs a lot less indirection.

It may be useless complexity, but we don't have any benchmarks at the moment so I'd wait for that before making any refactoring.

acl-cqc · 2023-07-25T07:42:13Z

I have to wonder whether we need such control over efficient static allocation for test code

It's mostly useful when dealing with Signatures of leaf ops. OpType::signature on something like a CNOT does not need to allocate at all, so checking things on flat dataflow regions needs a lot less indirection.

It may be useless complexity, but we don't have any benchmarks at the moment so I'd wait for that before making any refactoring.

Just to clarify - I did not mean to suggest removal of type_row!. [You give a good example of which I was not aware, to boot!] However, I wonder whether there is really value in having classic_row! as well. Uses thereof outside tests amount to two, in simple.rs, both times empty i.e. classic_row![]. Maybe we can revisit when/if we make predicates simple rather than classic...

…_typerow_classic_predicate

acl-cqc added 23 commits July 7, 2023 13:26

In ConstValue::predicate, assert that the unit value is expected by t…

ee60c72

…he type

Remove one TypeRow::from, inlining into the other

900ffce

Linear containers can have classic values too

5455cd9

Flatten LinearType into SimpleType

de40c36

Inline single use of impl_from_into_simple_type macro

4d585f8

Merge remote-tracking branch 'origin/main' into refactor/linear_classic

079dd6f

WIP

6ccc5fa

Masses of ToOwned/'static weirdness

68772ba

Require Debug everywhere

757426f

More ToOwned - everything <Owned=Vec<T>>

ccfdb13

TypeRow<ClassicType> has predicate_variant_rows as predicates are alw…

76616a0

…ays Classical

try_convert_elems for new_sum/new_tuple

edbbdf8

Driveby use SimpleType::is_linear rather than lambda

b5b3975

TEMP turn off types/simple/serialize.rs

c48e4e7

Constant Sums must be all-classic; other fixes

5b080a2

Add TypeRow::into_owned; Remove ConstTypeError::LinearTypeDisallowed;…

83b9736

… cargo build compiles

Fix tests, except serialization of course!

93b030a

T: Clone+'static avoids the crazy [T]: ToOwned<Owned=Vec<T>>

4c8141a

Use PrimType constraint for all TypeRow's

a2b4f2c

Revert "TEMP turn off types/simple/serialize.rs"

8ef8399

This reverts commit c48e4e7.

Fix serialization by making SerSimpleType contain only SerSimpleTypes

d6a7452

Drop a now-unnecessary into

6c01233

Remove now-unnecessary hoop-jumping in Signature::get for 'other' ports

9e43ddb

acl-cqc changed the base branch from main to refactor/linear_classic July 10, 2023 13:53

acl-cqc commented Jul 10, 2023

View reviewed changes

src/types/simple/serialize.rs Outdated Show resolved Hide resolved

acl-cqc commented Jul 10, 2023

View reviewed changes

src/types/simple.rs Show resolved Hide resolved

acl-cqc added 2 commits July 12, 2023 15:42

Merge remote-tracking branch 'origin/main' into refactor/linear_classic

dd6c705

Merge remote-tracking branch 'origin/main' into HEAD

2e4a0bb

acl-cqc added 3 commits July 17, 2023 12:06

Use unicode superset

48e4e39

typo: anynum -> any

230adf4

Merge remote-tracking branch 'origin/main' into refactor/linear_classic

a469f3b

Base automatically changed from refactor/linear_classic to main July 17, 2023 11:21

acl-cqc added 13 commits July 17, 2023 12:23

Merge remote-tracking branch 'origin/main' into refactor/linear_classic

e852511

Merge branch 'refactor/linear_classic' into refactor/parameterize_typ…

40cfd79

…erow_classic_predicate

Use classic_row!

5e90a5c

SerializableType trait w/CLASSIC, unimplemented for SerSimpleType

3a6ec73

A couple more classic_row!s

3061a5f

Drop some unnecessary type annotations (add comments)

fdf88ba

SimpleRow + ClassicRow

9a11e07

fix doc links in macros.rs

8696dad

Properly doc classic_row!

e04294c

And more macro doc fixes

d8023f9

Merge remote-tracking branch 'origin/main' into refactor/parameterize…

ae63688

…_typerow_classic_predicate

fmt

c71f6fe

fix doc

fecce95

acl-cqc requested a review from aborgna-q July 24, 2023 08:16

acl-cqc mentioned this pull request Jul 24, 2023

Add HashableType as subset of ClassicType #283

Merged

Test new_tuple and new_sum

c32bd45

aborgna-q approved these changes Jul 24, 2023

View reviewed changes

acl-cqc added 3 commits July 24, 2023 21:00

Combine impl blocks

b6f023c

types: export PrimType, TypeRow

ea976ba

tidy some imports

f4dcd89

Merge remote-tracking branch 'origin/main' into refactor/parameterize…

f74a968

…_typerow_classic_predicate

acl-cqc merged commit 2045c97 into main Jul 25, 2023
6 checks passed

acl-cqc deleted the refactor/parameterize_typerow_classic_predicate branch July 25, 2023 08:43

acl-cqc mentioned this pull request Jul 26, 2023

Allow linear elements in predicates #253

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parameterize TypeRow by element type #256

Parameterize TypeRow by element type #256

acl-cqc commented Jul 10, 2023 •

edited

Loading

acl-cqc Jul 10, 2023

acl-cqc Jul 10, 2023

aborgna-q left a comment

aborgna-q Jul 24, 2023

acl-cqc Jul 24, 2023

acl-cqc Jul 25, 2023

aborgna-q Jul 24, 2023

aborgna-q commented Jul 24, 2023

acl-cqc commented Jul 25, 2023

Parameterize TypeRow by element type #256

Parameterize TypeRow by element type #256

Conversation

acl-cqc commented Jul 10, 2023 • edited Loading

acl-cqc Jul 10, 2023

Choose a reason for hiding this comment

acl-cqc Jul 10, 2023

Choose a reason for hiding this comment

aborgna-q left a comment

Choose a reason for hiding this comment

aborgna-q Jul 24, 2023

Choose a reason for hiding this comment

acl-cqc Jul 24, 2023

Choose a reason for hiding this comment

acl-cqc Jul 25, 2023

Choose a reason for hiding this comment

aborgna-q Jul 24, 2023

Choose a reason for hiding this comment

aborgna-q commented Jul 24, 2023

acl-cqc commented Jul 25, 2023

acl-cqc commented Jul 10, 2023 •

edited

Loading