Skip to content

Conversation

@sgilmore10
Copy link
Member

@sgilmore10 sgilmore10 commented May 8, 2023

Rationale for this change

Continuing to build the MATLAB interface to Arrow for numeric types. We're trying to keep the scope of these pull requests small to make them easier to review.

What changes are included in this PR?

  1. Added arrow.array.Float32Array MATLAB class
  2. Added new test class tFloat32Array.m

Are these changes tested?

  1. Yes, added a new test class called tFloat32Array.m.
  2. Qualified locally on macOS.

Are there any user-facing changes?

Yes, users can now create a array of float32 values.

Future Directions

  1. Continue building out support for numeric arrays.
  2. Add shared validation functions.
  3. Make a shared test class for Numeric Arrays.

@github-actions
Copy link

github-actions bot commented May 8, 2023

Co-authored-by: Kevin Gurney <kgurney@mathworks.com>
@sgilmore10 sgilmore10 marked this pull request as ready for review May 12, 2023 13:16
@sgilmore10 sgilmore10 requested a review from assignUser as a code owner May 12, 2023 13:16
@sgilmore10
Copy link
Member Author

Marking this as ready for review.

@sgilmore10
Copy link
Member Author

I'm not sure why there was a linker error in the macOS build. The MATLAB action passed in the mathworks/arrow fork. I noticed a couple of the other MATLAB runs in mathworks/arrow have failed with the same linker error, but it the issue appears to be sporadic. I'll try re-running the action to see if it the same issue appears again.

Copy link
Member

@kou kou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

I think that we can merge this after we fix the CI problem.

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting review Awaiting review labels May 15, 2023
sgilmore10 and others added 2 commits May 15, 2023 09:01
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
@sgilmore10
Copy link
Member Author

Hi @kou,

The CI passed on the latest try. It looks like the CI sporadic and not related to this pull request because we've seen a couple of times now.

@kou
Copy link
Member

kou commented May 15, 2023

OK.
I'll merge this.

@kou kou merged commit 660cb6e into apache:main May 15, 2023
@sgilmore10
Copy link
Member Author

Thanks for merging this @kou! We're going to look into this failure in the background.

@sgilmore10
Copy link
Member Author

We've captured this sporadic build failure as an issue in mathworks/libmexclass.

ArgusLi pushed a commit to Bit-Quill/arrow that referenced this pull request May 15, 2023
…pache#35495)

### Rationale for this change
Continuing to build the MATLAB interface to Arrow for numeric types. We're trying to keep the scope of these pull requests small to make them easier to review. 

### What changes are included in this PR?

1. Added `arrow.array.Float32Array` MATLAB class
2. Added new test class `tFloat32Array.m`

### Are these changes tested?

1. Yes, added a new test class called `tFloat32Array.m`. 
2. Qualified locally on macOS.

### Are there any user-facing changes?

Yes, users can now create a array of float32 values.

### Future Directions

1. Continue building out support for numeric arrays.
2. Add shared validation functions.
3. Make a shared test class for Numeric Arrays.
* Closes: apache#35492

Lead-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Co-authored-by: sgilmore10 <74676073+sgilmore10@users.noreply.github.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Co-authored-by: Kevin Gurney <kgurney@mathworks.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@ursabot
Copy link

ursabot commented May 16, 2023

Benchmark runs are scheduled for baseline = 5ebd4c6 and contender = 660cb6e. 660cb6e is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed ⬇️0.55% ⬆️0.0%] test-mac-arm
[Finished ⬇️0.76% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.54% ⬆️0.0%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] 660cb6ef ec2-t3-xlarge-us-east-2
[Failed] 660cb6ef test-mac-arm
[Finished] 660cb6ef ursa-i9-9960x
[Finished] 660cb6ef ursa-thinkcentre-m75q
[Finished] 5ebd4c61 ec2-t3-xlarge-us-east-2
[Failed] 5ebd4c61 test-mac-arm
[Finished] 5ebd4c61 ursa-i9-9960x
[Finished] 5ebd4c61 ursa-thinkcentre-m75q
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

@ursabot
Copy link

ursabot commented May 16, 2023

['Python', 'R'] benchmarks have high level of regressions.
ursa-i9-9960x

kou pushed a commit that referenced this pull request May 19, 2023
…nt8Array`, `UInt16Array`, `UInt32Array`, `UInt64Array`) (#35562)

### Rationale for this change
Followup to #35495 in which we added the MATLAB class `Float32Array`.

This pull request adds support for round tripping signed integer between `arrow.array.<Type>Array` classes and associated MATLAB types (e.g. `uint8`, `uint16`, `uint32`, `uint64`).

| Arrow Array Type | MATLAB Type |
| ----------------- | -------------- |
| `UInt8Array` | `uint8`|
| `UInt16Array` | `uint16`|
| `UInt32Array` | `uint32`|
| `UInt64Array` | `uint64`|

Example of round-tripping `uint8` data:

```matlab
>> uint8MATLABArray = uint8([1 2 3]')

uint8MATLABArray =

  3×1 uint8 column vector

   1
   2
   3

>> uint8ArrowArray = arrow.array.UInt8Array(uint8MATLABArray)

uint8ArrowArray = 

[
  1,
  2,
  3
]

>> uint8MATLABArrayRoundTripped = toMATLAB(uint8ArrowArray)

uint8MATLABArrayRoundTripped =

  3×1 uint8 column vector

   1
   2
   3

>> all(uint8MATLABArray == uint8MATLABArrayRoundTripped)

ans =

  logical

   1
```

### What changes are included in this PR?
Added four new unsigned integer type `arrow.array.<Array>` concrete subclasses.

1. `arrow.array.UInt8Array`
2. `arrow.array.UInt16Array`
3. `arrow.array.UInt32Array`
4. `arrow.array.UInt64Array`

### Are these changes tested?

Yes, we added the following four test classes:

1. `tUInt8Array.m`
2. `tUInt16Array.m`
3. `tUInt32Array.m`
4. `tUInt64Array.m`

### Are there any user-facing changes?

Yes. This change introduces 4 new publicly documented classes:

1. `arrow.array.UInt8Array`
2. `arrow.array.UInt16Array`
3. `arrow.array.UInt32Array`
4. `arrow.array.UInt64Array`

### Future Directions

1. Add support for null values (i.e. validity bitmap) for the unsigned integer array types.

### NOTES

1. Thank you to @ kevingurney for his help with this PR!
* Closes: #35557

Lead-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Co-authored-by: Kevin Gurney <kgurney@mathworks.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
kou pushed a commit that referenced this pull request May 22, 2023
…8Array`, `Int16Array`, `Int32Array`, `Int64Array`) (#35561)

### Rationale for this change

Followup to #35495 in which we added the MATLAB class `Float32Array`.

This pull request adds support for round tripping signed integer between `arrow.array.<Array>` classes and associated MATLAB types (e.g. `int8`, `int16`, `int32`, `int64`).

| Arrow Array Type | MATLAB Type |
| ------------------------- | -------------------- |
| `Int8Array`             | `int8`                |
| `Int16Array`             | `int16`                |
| `Int32Array`             | `int32`                |
| `Int64Array`             | `int64`                |

Example of round-tripping `int8` data:

```matlab
>> int8MatlabArray = int8([1, 2, 3]')

int8MatlabArray =

  3x1 int8 column vector

   1
   2
   3

>> int8ArrowArray = arrow.array.Int8Array(int8MatlabArray) 

int8ArrowArray = 

[
  1,
  2,
  3
]

>> int8MatlabArrayRoundTripped = toMATLAB(int8ArrowArray)

int8MatlabArrayRoundTripped =

  3x1 int8 column vector

   1
   2
   3

>> all(int8MatlabArray == int8MatlabArrayRoundTripped)

ans =

  logical

   1

```

### What changes are included in this PR?

Added four new signed integer type `arrow.array.<Array>` concrete subclasses.

1.  `arrow.array.Int8Array`
3. `arrow.array.Int16Array`
4. `arrow.array.Int32Array` 
5. `arrow.array.Int64Array` 

### Are these changes tested?

Yes, we added the following four test classes:

1.  `tInt8Array.m`
2. `tInt16Array.m`
3. `tInt32Array.m`
4. `tInt64Array.m`

### Are there any user-facing changes?

Yes. This change introduces 4 new publicly documented classes:

1.  `arrow.array.Int8Array`
3. `arrow.array.Int16Array`
4. `arrow.array.Int32Array` 
5. `arrow.array.Int64Array` 

### Future Directions

1. Add support for null values (i.e. validity bitmap) for the signed integer array types.

### Notes

!. Thank you to @ sgilmore10 for her help with this pull request!
* Closes: #35558

Lead-authored-by: Kevin Gurney <kgurney@mathworks.com>
Co-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@sgilmore10 sgilmore10 deleted the GH-35492 branch June 27, 2023 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[MATLAB] Add arrow.array.Float32Array MATLAB Class

3 participants