feat!: make (most) vecops take a matrix instead, add matrix literals #714
Conversation
Change the types of most operations that take vectors to take a single matrix argument. This aligns them with Savile Row, and allows them to be used with matrix variables, as well as a literal list of items. This excludes sum and product. To enable this, this PR adds: + Parsing of matrix literals with and without implicit domains. + Add macros `matrix`, `into_matrix`, `matrix_expr`, `into_matrix_expr` for the creation of matrix literals. + Add the concept of a list, `unwrap_list`, and the `matrix_to_list` rule. + Add `Range::UnboundedR` and `Range::UnboundedL` + Refactor affected rules and tests to use `unwrap_list` and `matrix_expr` / `into_matrix_expr` instead of vectors. LISTS Many of our rules involve changing the number of arguments inside a matrix. For example, the partial evaluation of `or`: ``` or([a,b,false,false]) ~> or([a,b]) ``` In general, it is not valid to change the size of a matrix as this is part of their domain. Also, matrices can have non-contiguous index domains, and what happens to these when elements are removed are non-obvious. For example, the transformation ``` or([a,b,false,false;int(2,4,6,8)]) ~> or([a,b;int(2,4,6,8)]) ``` is invalid, as the matrix on the right has more indices than elements. A list is a matrix with the domain `int(1..)`. This domain is unbounded, so allows arbitrary element addition and removal. When a matrix literal `[a,b,c]` is constructed without an explicit domain (e.g. using `matrix_expr!`), it is assumed to be a list, and given the domain `int(1..)`. If an explicit domain is given (e.g. `[a,b,c;int(1,2,4)]`, that is used instead. In rules, `unwrap_list` can be used to check if an expression is a list or not, and `matrix_expr!` / `into_matrix_expr!` can be used to construct a list from its elements. Most rules except matrix indexing / slicing should work on lists only. MATRIX_TO_LIST RULE Conjure gives `[a,b,c]` the domain `int(1..3)`. Our Conjure-JSON parser inherits this behaviour. Under our definition, this is not a list, so most rules cannot apply to it. However, as long as it is not inside another matrix (e.g. is a row in a 2d matrix), and has a single contiguous domain `int(1..n)`, it is safe to treat it as such. `matrix_to_rule` performs this conversion. For example, ``` min([1,2,3,4;int(1..4)]) ~~> min([1,2,3,4;int(1..)] ``` If we instead had ``` min([[1,2,3,4;int(1..4)],[5,6,7,8;int(1..4)];int(1..2)] ``` `matrix_to_rule` would not apply to `[1,2,3,4]`; this ensures that all the rows of the matrix remain the same size. However, it could apply to the containing matrix. For the exact conditions this rule runs under, see the doc comment for `matrix_to_rule`.
niklasdewally
force-pushed
the
nik/matrix/make-vecops-take-matrices
branch
from
c8e1600 to
ee50174
Compare
|
Apologies for the large diff! @ozgurakgun Things are mostly as they were in our meeting earlier today, except for finishing the parsing code and adding A lot of this PR is refactoring the uses of vecops to use matrices instead; the important changes that add the new functionality are in |
|
looks good. not hugely important, but in a few places in the description you say the domain of the matrix when you refer to what I would call the index domain. [1,2,3;int(4..6)] has domain matrix indexed by [int(4..6)] of int(1..3), and it's index domains are [int(4..6)] |
Fixed; thanks. |
…terals feat!: make (most) vecops take a matrix instead, add matrix literals Change the types of most operations that take vectors to take a single matrix argument. This aligns them with Savile Row, and allows them to be used with matrix variables, as well as a literal list of items. This excludes sum and product. To enable this, this PR adds: + Parsing of matrix literals with and without implicit index domains. + Add macros `matrix`, `into_matrix`, `matrix_expr`, `into_matrix_expr` for the creation of matrix literals. + Add the concept of a list, `unwrap_list`, and the `matrix_to_list` rule. + Add `Range::UnboundedR` and `Range::UnboundedL` + Refactor affected rules and tests to use `unwrap_list` and `matrix_expr` / `into_matrix_expr` instead of vectors. LISTS Many of our rules involve changing the number of arguments inside a matrix. For example, the partial evaluation of `or`: ``` or([a,b,false,false]) ~> or([a,b]) ``` In general, it is not valid to change the size of a matrix as this is part of their index domain. Also, matrices can have non-contiguous index domains, and what happens to these when elements are removed are non-obvious. For example, the transformation ``` or([a,b,false,false;int(2,4,6,8)]) ~> or([a,b;int(2,4,6,8)]) ``` is invalid, as the matrix on the right has more indices than elements. A list is a matrix with the index domain `int(1..)`. This domain is unbounded, so allows arbitrary element addition and removal. When a matrix literal `[a,b,c]` is constructed without an explicit index domain (e.g. using `matrix_expr!`), it is assumed to be a list, and given the index domain `int(1..)`. If an explicit index domain is given (e.g. `[a,b,c;int(1,2,4)]`, that is used instead. In rules, `unwrap_list` can be used to check if an expression is a list or not, and `matrix_expr!` / `into_matrix_expr!` can be used to construct a list from its elements. Most rules except matrix indexing / slicing should work on lists only. MATRIX_TO_LIST RULE Conjure gives `[a,b,c]` the index domain `int(1..3)`. Our Conjure-JSON parser inherits this behaviour. Under our definition, this is not a list, so most rules cannot apply to it. However, as long as it is not inside another matrix (e.g. is a row in a 2d matrix), and has a single contiguous index domain `int(1..n)`, it is safe to treat it as such. `matrix_to_rule` performs this conversion. For example, ``` min([1,2,3,4;int(1..4)]) ~~> min([1,2,3,4;int(1..)] ``` If we instead had ``` min([[1,2,3,4;int(1..4)],[5,6,7,8;int(1..4)];int(1..2)] ``` `matrix_to_rule` would not apply to `[1,2,3,4]`; this ensures that all the rows of the matrix remain the same size. However, it could apply to the containing matrix. For the exact conditions this rule runs under, see the doc comment for `matrix_to_rule`.
Code and Documentation Coverage ReportDocumentation CoverageThis PR: conjure_core: 3% with examples, 54% documented -- 8/153/296 Main: conjure_core: 3% with examples, 52% documented -- 8/147/290 View full documentation coverage for main, this PR Code Coverage SummaryThis PR: Detailed Report Main: Detailed Report Coverage Main & PR Coverage ChangeFunctions coverage changed by 0.70% and covered lines changed by 21
Branches... coverage: No comparison data available |
To clarify, is this ready to merge? |
|
I hadn't fully checked at the time, but did so far, please feel free to merge! |
Thanks! |
|
@Shikhar-Srivastava-16 - this is the relavant PR. |
Change the types of most operations that take vectors to take a single matrix argument. This aligns them with Savile Row, and allows them to be used with matrix variables, as well as a literal list of items. This excludes sum and product.
To enable this, this PR adds:
Parsing of matrix literals with and without explicit index domains.
Add macros
matrix,into_matrix,matrix_expr,into_matrix_exprfor the creation of matrix literals.Add the concept of a list,
unwrap_list, and thematrix_to_listrule.Add
Range::UnboundedRandRange::UnboundedLRefactor affected rules and tests to use
unwrap_listandmatrix_expr/into_matrix_exprinstead of vectors.Lists
Many of our rules involve changing the number of arguments inside a matrix. For example, the partial evaluation of
or:In general, it is not valid to change the size of a matrix as this is part of their index domain. Also, matrices can have non-contiguous index domains, and what happens to these when elements are removed are non-obvious.
For example, the transformation
is invalid, as the matrix on the right has more indices than elements.
A list is a matrix with the index domain
int(1..). This domain is unbounded, so allows arbitrary element addition and removal.When a matrix literal
[a,b,c]is constructed without an explicit index domain (e.g. usingmatrix_expr!), it is assumed to be a list, and given the index domainint(1..). If an explicit index domain is given (e.g.[a,b,c;int(1,2,4)], that is used instead.In rules,
unwrap_listcan be used to check if an expression is a list or not, andmatrix_expr!/into_matrix_expr!can be used to construct a list from its elements.Most rules except matrix indexing / slicing should work on lists only.
matrix_to_listRuleConjure gives
[a,b,c]the index domainint(1..3). Our Conjure-JSON parser inherits this behaviour.Under our definition, this is not a list, so most rules cannot apply to it. However, as long as it is not inside another matrix (e.g. is a row in a 2d matrix), and has a single contiguous index domain
int(1..n), it is safe to treat it as such.matrix_to_ruleperforms this conversion.For example,
If we instead had
matrix_to_rulewould not apply to[1,2,3,4]; this ensures that all the rows of the matrix remain the same size. However, it could apply to the containing matrix.For the exact conditions this rule runs under, see the doc comment for
matrix_to_rule.