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.

Sign up for GitHub

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

Jump to bottom

[SYSTEMDS-3589] Frame single column ragged array #1857

Closed

OlgaOvcharenko wants to merge 9 commits into apache:main from OlgaOvcharenko:ragged_array

Contributor

OlgaOvcharenko commented Jul 3, 2023

This PR adds a ragged array for a single column containing row values. It is just a wrapper around actual frame column Arrays.
It can be used for the transformencode metadata frames.

Thanks for the review.

OlgaOvcharenko added 2 commits

July 3, 2023 16:20


          Add single column ragged array

c270404


          Add tests and fixes

75dc116

Contributor

phaniarnab commented Jul 3, 2023

I don't fully understand the use case of a ragged array-based frame. Is the metatadata from a transformencode the only case where this feature helps? Will splitting a metadata frame into a list of frames (one per column) not serve the purpose?

Contributor Author

OlgaOvcharenko commented Jul 4, 2023

I don't fully understand the use case of a ragged array-based frame. Is the metatadata from a transformencode the only case where this feature helps? Will splitting a metadata frame into a list of frames (one per column) not serve the purpose?

The ragged array is intended to save mem since the metadata frame is mostly nulls. We probably do not want to change the API of transformencode; therefore, I do not think a list of frames would be a suitable solution. This change makes no outside API changes and reduces memory allocation and the on-disk size of the metadata that we save after transformencode.

OlgaOvcharenko added 4 commits

July 4, 2023 13:03


          Add single column ragged array

2f5cd6d


          Add tests and fixes

d556c99


          Merge branch 'ragged_array' of github.com:OlgaOvcharenko/systemds int…

529fc80

…o ragged_array


          Fix DDC test init

8480c42

Baunsgaard reviewed

View reviewed changes

Contributor

Baunsgaard left a comment

good start, some details missing.

src/main/java/org/apache/sysds/runtime/frame/data/columns/RaggedArray.java Outdated Show resolved Hide resolved

src/main/java/org/apache/sysds/runtime/frame/data/columns/RaggedArray.java Outdated Show resolved Hide resolved

src/main/java/org/apache/sysds/runtime/frame/data/columns/RaggedArray.java Outdated

    
              	}

              	@Override

              	public Object get() {

              		throw new NotImplementedException("Unimplemented method 'get'");

              		return _a.get();

Contributor

Baunsgaard Jul 5, 2023

throw exception on this one. since the API expects to get the full array of size m.

Contributor Author

OlgaOvcharenko Jul 11, 2023

Fixed. However, OptionalArray.get returns an Object and it makes sense to me.

src/main/java/org/apache/sysds/runtime/frame/data/columns/RaggedArray.java Outdated Show resolved Hide resolved

src/main/java/org/apache/sysds/runtime/frame/data/columns/RaggedArray.java Show resolved Hide resolved

src/main/java/org/apache/sysds/runtime/frame/data/columns/RaggedArray.java Outdated

    
              	}

              	@Override

              	public byte[] getAsByteArray() {

              		throw new NotImplementedException("Unimplemented method 'getAsByteArray'");

              		return _a.getAsByteArray();

Contributor

Baunsgaard Jul 5, 2023

throw some exception here. this will not work, since the caller expects the byte array to be certain m size * nBytes per value.

Contributor Author

OlgaOvcharenko Jul 11, 2023

Then, I believe that optional should also throw an exception to be consistent?
Since currently it is not the case:

public byte[] getAsByteArray() {
     // technically not correct.
    return _a.getAsByteArray();
}

Contributor

Baunsgaard Jul 11, 2023

yes optional should throw exception as well.

src/main/java/org/apache/sysds/runtime/frame/data/columns/RaggedArray.java Outdated

    
              	}

              	@Override

              	public long getExactSerializedSize() {

              		throw new NotImplementedException("Unimplemented method 'getExactSerializedSize'");

              		return _a.getExactSerializedSize();

Contributor

Baunsgaard Jul 5, 2023

int size.

src/main/java/org/apache/sysds/runtime/frame/data/columns/RaggedArray.java Show resolved Hide resolved

src/main/java/org/apache/sysds/runtime/frame/data/columns/RaggedArray.java Show resolved Hide resolved

src/test/java/org/apache/sysds/test/component/frame/array/FrameArrayTests.java Show resolved Hide resolved

OlgaOvcharenko added 3 commits

July 11, 2023 13:13


          Fix serialization

0723a02


          Fix size

79c2549


          Address comments

2a52580

Baunsgaard pushed a commit to Baunsgaard/systemds that referenced this pull request


          [SYSTEMDS-3589] Frame single column ragged array

504d2bf

This commit contains code to add a simple ragged array, that allows us
to allocate columns in frames with a lower number of contained materialized
values.

Closes apache#1857

Baunsgaard mentioned this pull request

[SYSTEMDS-3589] Frame single column ragged array #1884

Closed

Contributor

Baunsgaard commented Aug 21, 2023

Thanks for the commit, i am merging and fixing remaining issues in other PR.

Baunsgaard closed this

Baunsgaard pushed a commit that referenced this pull request


          [SYSTEMDS-3589] Frame single column ragged array

f094eac

This commit contains code to add a simple ragged array, that allows us
to allocate columns in frames with a lower number of contained materialized
values.

Closes #1857
Closes #1884

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment