Add Polars DataFrame Support for Dataset #2029

ragyabraham · 2024-07-17T03:21:47Z

This PR enhances the library's flexibility by allowing users to leverage Polars DataFrames for training purposes.

Checklist

Executed run-checks all script
Updated documentation to reflect changes in this PR

Related Issues/PRs

Fixes #2015

Changes

Added ability to create a burn dataset directly from a Polars DataFrame for training
Added a deserializer to deserialize df row into a user provided struct.
Implemented dataframe.rs module in the burn-dataset crate under the dataframe feature flag
Updated dataset document

Testing

Added additional unit tests to cover new functionality

…-dataframe

…t polars rows to burn dataset values

ragyabraham · 2024-07-17T03:25:18Z

@antimora

antimora

I have a few inlined comments and a question.

Polars has serde (feature) support for dataframes (see code). Probably you can bypass using JSON to deserialize row altogether. Have you looked into this?

crates/burn-dataset/Cargo.toml

crates/burn-dataset/src/dataset/mod.rs

ragyabraham · 2024-07-17T04:25:32Z

I have a few inlined comments and a question.

Polars has serde (feature) support for dataframes (see code). Probably you can bypass using JSON to deserialize row altogether. Have you looked into this?

Hey @antimora, I did, but I found that the serialize method is not fully implemented in polars yet. I've opened an issue on the Polars repo.

antimora · 2024-07-17T04:38:46Z

I have a few inlined comments and a question.
Polars has serde (feature) support for dataframes (see code). Probably you can bypass using JSON to deserialize row altogether. Have you looked into this?

Hey @antimora, I did, but I found that the serialize method is not fully implemented in polars yet. I've opened an issue on the Polars repo.

If I understood correctly not all column types supported? Can we support a subset of possible types? This still would be highly useful. Otherwise it seems one needs to encode row into JSON, which won't much more useful than having json row encoded lines in a plain text file. Or maybe I am misunderstanding the use of JSON here.

ragyabraham · 2024-07-17T08:16:21Z

I have a few inlined comments and a question.
Polars has serde (feature) support for dataframes (see code). Probably you can bypass using JSON to deserialize row altogether. Have you looked into this?

Hey @antimora, I did, but I found that the serialize method is not fully implemented in polars yet. I've opened an issue on the Polars repo.

If I understood correctly not all column types supported? Can we support a subset of possible types? This still would be highly useful. Otherwise it seems one needs to encode row into JSON, which won't much more useful than having json row encoded lines in a plain text file. Or maybe I am misunderstanding the use of JSON here.

it seems that the pull request to fix the todo! issue in polars might get through. if that's the case then we can just utilise that. will be cleaner

antimora · 2024-07-17T15:59:38Z

I have a few inlined comments and a question.
Polars has serde (feature) support for dataframes (see code). Probably you can bypass using JSON to deserialize row altogether. Have you looked into this?

Hey @antimora, I did, but I found that the serialize method is not fully implemented in polars yet. I've opened an issue on the Polars repo.

If I understood correctly not all column types supported? Can we support a subset of possible types? This still would be highly useful. Otherwise it seems one needs to encode row into JSON, which won't much more useful than having json row encoded lines in a plain text file. Or maybe I am misunderstanding the use of JSON here.

it seems that the pull request to fix the todo! issue in polars might get through. if that's the case then we can just utilise that. will be cleaner

Yeah I think we should definitely use Dataframe's native serialization/deserialization methods. Otherwise this Dataset will be little of use.

A cool use case I envision as follows:

One can open any Polars datasource (CVS, JSON, binaries, SQL Dbs, etc) into Dataframe.
Apply dataframe filters to filter (
Define struct for a row
Wrap with Dataframe Dataset (can also be passed to SqliteDataset or MemoryDataset to consume and cache locall)
Pass it to training/testing

BTW, your PR is waiting to fix this. Not sure if you noticed:

… map to json string. added comments

antimora

We spoke offline about using Polar's serde feature to deserialize a row. I am not sure if new code wasn't pushed (that you mentioned working), or there is still misunderstanding.

crates/burn-dataset/Cargo.toml

crates/burn-dataset/src/dataset/dataframe.rs

ragyabraham · 2024-07-18T20:01:03Z

We spoke offline about using Polar's serde feature to deserialize a row. I am not sure if new code wasn't pushed (that you mentioned working), or there is still misunderstanding.

The code that I showed you working is the code in this PR as it stands

…method

…. no longer convert to object before converting to string. no longer using serde_json to_string method

…gyabraham/burn into pr/2029

antimora · 2024-07-19T05:42:19Z

OK. I removed serde_json and implemented the native deserializer. Please finish up and clean up the code. Also please add more tests and data types.

One test is passing:

[burn-dataset]% cargo test dataframe --features dataframe
    Finished `test` profile [unoptimized] target(s) in 0.12s
     Running unittests src/lib.rs (/Users/dilshod/Projects/burn/target/debug/deps/burn_dataset-c350f307979d05bd)

running 1 test
test dataset::dataframe::tests::test_dataframe_dataset ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 25 filtered out; finished in 0.00s

… data. added a few more tests

ragyabraham · 2024-07-28T07:11:27Z

OK. I removed serde_json and implemented the native deserializer. Please finish up and clean up the code. Also please add more tests and data types.

One test is passing:

[burn-dataset]% cargo test dataframe --features dataframe
    Finished `test` profile [unoptimized] target(s) in 0.12s
     Running unittests src/lib.rs (/Users/dilshod/Projects/burn/target/debug/deps/burn_dataset-c350f307979d05bd)

running 1 test
test dataset::dataframe::tests::test_dataframe_dataset ... ok

test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 25 filtered out; finished in 0.00s

@antimora I've added support for LazyFrame, added support to deserialize a few more datatypes and added some tests. So far, there's no serialization for complex types, but this is a good starting point. Let me know what you think and if there's anything urgent I need to fix/add

Two tests are passing

antimora

Let's remove Lazy for now. I thought it would be less complex and quick to add it but as it stands it's not complete and I don't want to burden you with this feature, sorry.

We need a couple minor/quick fixes. It should be ready to merge afterwards.

Also please rebase your branch to get the latest lock file.

crates/burn-dataset/src/dataset/dataframe.rs

antimora · 2024-07-31T17:10:59Z

@laggui @nathanielsimard I took over this PR to complete it. I made final changes and ready for your review.

codecov · 2024-07-31T18:34:24Z

Codecov Report

Attention: Patch coverage is 92.97125% with 22 lines in your changes missing coverage. Please review.

Project coverage is 86.19%. Comparing base (f673721) to head (3f81d53).

Files	Patch %	Lines
crates/burn-dataset/src/dataset/dataframe.rs	92.97%	22 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2029      +/-   ##
==========================================
+ Coverage   86.16%   86.19%   +0.02%     
==========================================
  Files         686      687       +1     
  Lines       87871    88184     +313     
==========================================
+ Hits        75717    76008     +291     
- Misses      12154    12176      +22

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

nathanielsimard

LGTM from my end. The deserializer logic is a bit complex, but the tests comparing with a vector dataset are reassuring. And I guess it's necessary with Polar, though I'm not familiar with the library.

antimora

Since I made the actual changes and fixes, I approve it.

wangjiawen2013 · 2024-08-25T12:58:10Z

Good!

ragyabraham and others added 5 commits July 16, 2024 15:51

initial commit to try implement from_dataframes for a burn dataset

3c8bc5c

Merge remote-tracking branch 'upstream/main' into dataset-from-polars…

c26eb8f

…-dataframe

added the beginnings of tests. removed ref to self in utility method

8e5ed1d

added unit test for dataframe module. added utility methods to conver…

a575988

…t polars rows to burn dataset values

Merge branch 'tracel-ai:main' into dataset-from-polars-dataframe

aa10285

antimora requested changes Jul 17, 2024

View reviewed changes

crates/burn-dataset/Cargo.toml Outdated Show resolved Hide resolved

crates/burn-dataset/src/dataset/mod.rs Outdated Show resolved Hide resolved

putting polars and dataframe mod behind a fearure flag

be30172

ragyabraham added 2 commits July 18, 2024 15:36

testing both methods

e704165

added a if let OK so that it doesn't panic. if we can't convert serde…

d5d7348

… map to json string. added comments

antimora requested changes Jul 18, 2024

View reviewed changes

ragyabraham and others added 8 commits July 19, 2024 08:48

using polars serializer, renaming vars

f787698

removed prints. just unwrapping

7c874e0

setting feature flags back

3decbd9

addressing some of the comments on PR. using native Polars serialize …

c182af5

…method

return Value::Null rather than panic if we can't serialize list value…

9e9b753

…. no longer convert to object before converting to string. no longer using serde_json to_string method

Use native deserializer instead of serde_json

c9549a7

Merge branch 'dataset-from-polars-dataframe' of https://github.com/ra…

275bd42

…gyabraham/burn into pr/2029

Merge branch 'main' into dataset-from-polars-dataframe

066b796

added support for lazyframes. added support to deserialize a few more…

84f9959

… data. added a few more tests

antimora requested changes Jul 29, 2024

View reviewed changes

antimora self-assigned this Jul 31, 2024

Remove lazy, add more testing and other fixes

fff4f5d

Update the book

0bdc677

antimora changed the title ~~[DRAFT] Dataset from polars dataframe~~ Add Polars DataFrame Support for Training Jul 31, 2024

Merge remote-tracking branch 'upstream/main' into pr/2029

d58dd7d

antimora marked this pull request as ready for review July 31, 2024 05:00

antimora added 2 commits July 31, 2024 00:05

Remove lazy feature

4404002

Merge remote-tracking branch 'upstream/main' into pr/2029

63e3af8

antimora requested review from laggui and nathanielsimard July 31, 2024 17:09

antimora changed the title ~~Add Polars DataFrame Support for Training~~ Add Polars DataFrame Support for Dataset Jul 31, 2024

Put back lazy feature for polars

490e326

nathanielsimard approved these changes Jul 31, 2024

View reviewed changes

Merge remote-tracking branch 'upstream/main' into pr/2029

3f81d53

antimora approved these changes Jul 31, 2024

View reviewed changes

antimora merged commit 04d7ff2 into tracel-ai:main Jul 31, 2024
14 checks passed

ragyabraham mentioned this pull request Aug 1, 2024

Implement Row Deserializer pola-rs/polars#17970

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Polars DataFrame Support for Dataset #2029

Add Polars DataFrame Support for Dataset #2029

ragyabraham commented Jul 17, 2024 •

edited by antimora

Loading

ragyabraham commented Jul 17, 2024

antimora left a comment

ragyabraham commented Jul 17, 2024

antimora commented Jul 17, 2024

ragyabraham commented Jul 17, 2024

antimora commented Jul 17, 2024

antimora left a comment

ragyabraham commented Jul 18, 2024

antimora commented Jul 19, 2024

ragyabraham commented Jul 28, 2024

antimora left a comment

antimora commented Jul 31, 2024

codecov bot commented Jul 31, 2024 •

edited

Loading

nathanielsimard left a comment

antimora left a comment

wangjiawen2013 commented Aug 25, 2024

Add Polars DataFrame Support for Dataset #2029

Add Polars DataFrame Support for Dataset #2029

Conversation

ragyabraham commented Jul 17, 2024 • edited by antimora Loading

Checklist

Related Issues/PRs

Changes

Testing

ragyabraham commented Jul 17, 2024

antimora left a comment

Choose a reason for hiding this comment

ragyabraham commented Jul 17, 2024

antimora commented Jul 17, 2024

ragyabraham commented Jul 17, 2024

antimora commented Jul 17, 2024

antimora left a comment

Choose a reason for hiding this comment

ragyabraham commented Jul 18, 2024

antimora commented Jul 19, 2024

ragyabraham commented Jul 28, 2024

antimora left a comment

Choose a reason for hiding this comment

antimora commented Jul 31, 2024

codecov bot commented Jul 31, 2024 • edited Loading

Codecov Report

nathanielsimard left a comment

Choose a reason for hiding this comment

antimora left a comment

Choose a reason for hiding this comment

wangjiawen2013 commented Aug 25, 2024

ragyabraham commented Jul 17, 2024 •

edited by antimora

Loading

codecov bot commented Jul 31, 2024 •

edited

Loading