[WIP] Add arrow from javacpp (also upgrades javacpp version) by agibsonccc · Pull Request #154 · KonduitAI/deeplearning4j

agibsonccc · 2019-12-29T10:18:53Z

What changes were proposed in this pull request?

Upgraded java cpp version (snapshots) for the arrow javacpp presets: https://github.com/bytedeco/javacpp-presets/tree/master/arrow
(Please fill in changes proposed in this fix)

How was this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)

Quick checklist

The following checklist helps ensure your PR is complete:

Eclipse Contributor Agreement signed, and signed commits - see IP Requirements page for details
Reviewed the Contributing Guidelines and followed the steps within.
Created tests for any significant new code additions.
Relevant tests for your changes are passing.

raver119

Generally LGTM, not sure about the need for additional datatypes in 1 method though.

raver119 · 2019-12-29T13:57:44Z

            case DOUBLE:
                Tensor.addTypeType(bufferBuilder,Type.Decimal);
                break;
+            case HALF:


We probably want to add other types here as well: bfloat16, LONG and unsigned types?

I don't think Arrow supports bfloat16. What do we do in those cases @AlexDBlack?

Yeah, it only has float16/32/64, no bfloat16
https://arrow.apache.org/docs/metadata.html
In those cases we'll need to convert, probably to float32 in the case of BFloat16

Ye, bfloat16 range is the same as float32. float16 range is way smaller.

@raver119 what do you think here? This was the official arrow bindings. I can migrate this over to the new one. See ByteDecoArrowSerde now.

…arning4j into ag_javacpparrow

…into ag_javacpparrow

…arning4j into ag_javacpparrow

saudet · 2020-01-21T08:30:04Z

FYI, I've finally got the builds working for Arrow with Flight, Gandiva, ORC, Parquet, and Plasma enabled on Linux, Mac, and Windows, so it's now all part of the presets: bytedeco/javacpp-presets@3cf0ef7

…into ag_javacpparrow

agibsonccc · 2020-02-02T02:38:41Z

@raver119 @saudet I updated this and made it run with the latest master. Besides CI, was else do we need to do here?

saudet · 2020-02-03T02:05:06Z

It looks alright now. Although the methods to convert arrays could probably be made faster with calls to GetValuesDouble(), etc as shown in this sample code: https://github.com/bytedeco/javacpp-presets/blob/master/arrow/samples/RowWiseConversionExample.java

- couple of stubs for CudaDataBuffer methods Signed-off-by: raver119 <raver119@gmail.com>

Signed-off-by: raver119 <raver119@gmail.com>

raver119 · 2020-04-07T10:00:17Z

I've fixed all ND4J tests for CPU and CUDA. DataVec side of things still looks incomplete. Tests should be testing something actually.

raver119

DataVec side of things looks incomplete. Tests do not actually test anything, besides of the execution fact etc

raver119 · 2020-04-07T11:07:46Z

Another thing to mention: datavec-arrow goes mad on JVM11 due to netty relying on Unsafe methods.

Add arrow from javacpp (also upgrades javacpp version)

eddf067

agibsonccc self-assigned this Dec 29, 2019

agibsonccc requested review from AlexDBlack, raver119 and saudet December 29, 2019 10:19

agibsonccc added the enhancement New feature or request label Dec 29, 2019

raver119 previously approved these changes Dec 29, 2019

View reviewed changes

Add basic table api

c472256

agibsonccc dismissed raver119’s stale review via c472256 December 29, 2019 14:09

AlexDBlack reviewed Dec 30, 2019

View reviewed changes

Add op runner test

62cb047

saudet suggested changes Dec 30, 2019

View reviewed changes

Comment thread nd4j/nd4j-serde/nd4j-arrow/src/main/java/org/nd4j/arrow/ByteDecoArrowSerde.java

Comment thread nd4j/nd4j-serde/nd4j-arrow/src/main/java/org/nd4j/arrow/ByteDecoArrowSerde.java Outdated

agibsonccc added 12 commits December 30, 2019 22:08

Fix byte pointer access

8749e1a

Merge branch 'master' into ag_javacpparrow

52ddfe6

Address comments

2e9e595

Merge branch 'ag_javacpparrow' of https://github.com/KonduitAI/deeple…

9158dfd

…arning4j into ag_javacpparrow

Initial table api

f03d2c5

@raver119 can you review this? String changes and adds WIP column api

faa004a

Auto infer num words for utf8 and add test

00f8fb8

Add more utilities

eb23dc4

Add tests for columns. Adds WIP row class.

4c60a54

All tests pass. Still need to fill out table and row.

64a0079

Make all curren tests pass.

a3a5de4

Add toNDArray and toList constructors/methods

38ed19c

saudet suggested changes Jan 7, 2020

View reviewed changes

Comment thread nd4j/nd4j-serde/nd4j-arrow/src/main/java/org/nd4j/arrow/ByteDecoArrowSerde.java

Comment thread nd4j/nd4j-serde/nd4j-arrow/src/main/java/org/nd4j/arrow/ByteDecoArrowSerde.java Outdated

raver119 reviewed Jan 7, 2020

View reviewed changes

Comment thread ...ls/nd4j-cuda/src/main/java/org/nd4j/linalg/jcublas/buffer/factory/CudaDataBufferFactory.java Outdated

agibsonccc added 4 commits January 8, 2020 11:22

Merge branch 'master' of https://github.com/KonduitAI/deeplearning4j …

5cdecd3

…into ag_javacpparrow

Get rid of merge conflicts

1538cae

Merge branch 'master' into ag_javacpparrow

40981a4

Get rid of compilation errors

c75a5e1

Merge branch 'ag_javacpparrow' of https://github.com/KonduitAI/deeple…

add666f

…arning4j into ag_javacpparrow

agibsonccc added 4 commits February 1, 2020 21:10

local

fd4c4ca

Merge branch 'master' of https://github.com/KonduitAI/deeplearning4j …

dacddb3

…into ag_javacpparrow

Make tests run post update

c9c29c2

Get rid of capacity/limit on arrow data

cca6af4

saudet previously approved these changes Feb 3, 2020

View reviewed changes

agibsonccc and others added 3 commits February 6, 2020 00:12

Merge branch 'master' into ag_javacpparrow

d1187e7

Merge branch 'master' into ag_javacpparrow

1b7de83

Merge branch 'master' into ag_javacpparrow

337f784

saudet requested a review from raver119 April 6, 2020 03:41

Merge branch 'master' into ag_javacpparrow

f3da385

saudet dismissed their stale review via f3da385 April 6, 2020 03:43

Merge branch 'master' into ag_javacpparrow

c7888f2

raver119 assigned raver119 and unassigned agibsonccc Apr 6, 2020

Merge branch 'master' into ag_javacpparrow

f43b4af

raver119 changed the title ~~Add arrow from javacpp (also upgrades javacpp version)~~ [WIP] Add arrow from javacpp (also upgrades javacpp version) Apr 7, 2020

raver119 added 5 commits April 7, 2020 08:32

- remove unwanted exception

ae9409f

- couple of stubs for CudaDataBuffer methods Signed-off-by: raver119 <raver119@gmail.com>

few fixes here and there to make nd4j tests pass without crashes

f8fdbd2

Signed-off-by: raver119 <raver119@gmail.com>

CUDA side of things updated

b32a573

Signed-off-by: raver119 <raver119@gmail.com>

Minor fix for Nd4jArrowOpRunner

71acf5b

Signed-off-by: raver119 <raver119@gmail.com>

minor test fix

f3cf1a0

Signed-off-by: raver119 <raver119@gmail.com>

raver119 assigned agibsonccc and unassigned raver119 Apr 7, 2020

raver119 suggested changes Apr 7, 2020

View reviewed changes

Conversation

agibsonccc commented Dec 29, 2019

What changes were proposed in this pull request?

How was this patch tested?

Quick checklist

Uh oh!

raver119 left a comment

Choose a reason for hiding this comment

Uh oh!

raver119 Dec 29, 2019

Choose a reason for hiding this comment

Uh oh!

saudet Dec 30, 2019

Choose a reason for hiding this comment

Uh oh!

AlexDBlack Dec 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

raver119 Dec 30, 2019

Choose a reason for hiding this comment

Uh oh!

agibsonccc Dec 30, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

saudet commented Jan 21, 2020

Uh oh!

agibsonccc commented Feb 2, 2020

Uh oh!

saudet commented Feb 3, 2020

Uh oh!

raver119 commented Apr 7, 2020

Uh oh!

raver119 left a comment

Choose a reason for hiding this comment

Uh oh!

raver119 commented Apr 7, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AlexDBlack Dec 30, 2019 •

edited

Loading