Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multi-dim arrays #20

Closed
KrisThielemans opened this issue Dec 4, 2022 · 2 comments
Closed

multi-dim arrays #20

KrisThielemans opened this issue Dec 4, 2022 · 2 comments

Comments

@KrisThielemans
Copy link

https://github.com/microsoft/yardl/blob/main/docs/docs.md#computed-fields states

MyRec: !record
  fields:
    arrayField: !array
        items: int
        dimensions: [x, y]
  computedFields:
    accessArrayElementByName: arrayField[y:1, x:0]

this swaps the order between dimensions and access. Is this intentional? It'd be very confusing!

Also, somewhere in the doc we'll need a description of mappings between yardl types and C++ and other target languages. In particular, I believe you generate your own multi-dim array type as there still doesn't seem to be an std container sadly.

It could be useful to support a few existing multi-dim arrays to avoid copies in client-code (Boost.MultiArray and https://amypad.github.io/CuVec/ come to mind), but I can see that becoming very difficult. (If a mapping to a flat array is exposed somewhere, it'd need to be stated if row-major or column-major order is used).

@johnstairs
Copy link
Member

I've opened a separate issue to discuss the C++ array library issue (#22).

Regarding the computed field syntax for accessing array values, it was a deliberate choice to allow the dimensions to be given in any order when they are specified by name. But I can see that being confusing. We can make a change to require them to always be provided in the correct order.

johnstairs added a commit that referenced this issue Dec 8, 2022
Based on feedback, we now require array access index arguments to be
given in the correct order, even when dimension names are used.

For example, this would no longer be valid:

```yaml
MyRec: !record
  fields:
    arrayField: !array
        items: int
        dimensions: [x, y]
  computedFields:
    accessArrayElementByName: arrayField[y:1, x:0]
```

Note that the `x` and `y` arguments are provided out of order. You will
now have to write `arrayField[x:0, y:1]`

Addresses #20
@johnstairs
Copy link
Member

Fixed in referenced PR. Will be part of next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants