*: framework agnostic preprocessing #781

tharvik · 2024-09-17T07:16:30Z

on the road to (closer by every PR) ONNX' land.

preprocessing revamp

simplifing preprocessing by using flat functions rather than classes
- allows to change type with every function rather than using the way-too-generic tf.TensorContainer
- for example, force the users to normalize the images before passing theses to a model by having the latter ingesting NormalizedImage
get rid of {Image,Tabular,Text}Data classes by replacing theses by correctly typed dataset

dataset improvements

add Dataset.cached to avoid recomputation when iterating multiple times
- is memory sensitive so can work with any dataset size (but not very useful if dataset materialization is bigger than RAM)
add preprocessOnce option to Disco to avoid all recomputation by keeping a preprocessed version of memory (more hardcore version of Dataset.cached)
- can crash the JS VM with OOM but yeah, let the user make the call
- quite useful to get reproducable timing for tests
Dataset.batch constructs batch concurrently

type safety

make Model, Task, Trainer, … generic on their datatype ("image" | "tabular" | "text") to check soundness at compile time
- ensure that when defining a Task<'tabular'>, is does have the required {input,output}Columns
- ensure that a {Disco,Trainer,Validator}<'image'> doesn't get feed a Dataset<{Tabular,Text}> but only a Dataset<[Image, label: string]>
single TrainingInformation<'tabular'>.outputColumn instead of multiple colum
- we in fact only supported single output but it was an array
clear separation between what is outside types ingested by Disco & Validator (Raw), and what is internal (ModelEncoded, after preprocessing and before postprocessing)

misc

change server/tests/status to not wait for changes on timeout but with promises in order to avoid slower computer timeouts
reduce discojs/validator/ to a single file
bump deps

sadly, there is a performance drawback to that: as the preprocessing computation is solely done in JS which is fundamentally monothread (as python is), processing is slower than the tfjs-node accelerated version we have. I tried a few ways to workaround it and push V8 to do magic but it didn't work. there is some way to make make all that work (#758) but that would require platform specific accelerators. and this PR was already a bit long so pushing it to another iteration.

JulienVig

Huge amount of work! You seem to be pushing typescript to its limits haha

I have an error when trying to build the webapp:

Error: Cannot find module @rollup/rollup-darwin-arm64. npm has a bug related to optional dependencies (https://github.com/npm/cli/issues/4828). Please try 'npm i again after removing both package-lock.json and node_modules directory.
at requireWithFriendlyError (/Users/vignoud/Disco/review/node_modules/rollup
/dist/native.js: 59:9)
at Object. ‹anonymous> (/Users/vignoud/Disco/review/node_modules/rollup/dist/
native. js: 68:76)
•. 3 lines matching cause stack trace ...
at Module._load (node:internal/modules/cjs/loader:1023:12) at cjsLoader (node:internal/modules/esm/translators:356:17)
at ModuleWrap. ‹anonymous> (node:internal/modules/esm/translators:305:7) at ModuleJob. run (node:internal/modules/esm/module_job:218:25)
at async ModuleLoader. import (node:internal/modules/esm/loader:329:24) {
  [cause]: Error: Cannot find module '@rollup/rollup-darwin-arm64'
  Require stack:
    - /Users/vignoud/Disco/review/node_modules/rollup/dist/native.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1144:15)
    at Module._load (node:internal/modules/cjs/loader:985:27) at Module. require (node:internal/modules/cjs/loader:1235:19)
    at require (node:internal/modules/helpers:176:18)
    at requireWithFriendlyError (/Users/vignoud/Disco/review/node_modules/roll up/dist/native.js:41:10)
    at Object. ‹anonymous> (/Users/vignoud/Disco/review/node_modules/rollup/dis
    t/native. js: 68:76)
    at Module._compile (node:internal/modules/cjs/loader:1376:14)
    at Module._extensions..js (node: internal/modules/cjs/Loader:1435:10) at Module. load (node:internal/modules/cjs/loader:1207:32)
    at Module._load (node:internal/modules/cjs/loader: 1023:12) {
    code: 'MODULE_NOT_FOUND' requireStack: [
    '/Users/vignoud/Disco/review/node_modules/rollup/dist/native.js'
    ]
  }
｝

I didn't manage to solve the issue even by re-creating the package-lock.json (it actually solves this error but then I can't build the server anymore because of new errors)

I couldn't review everything yet because of this but I already left a lot of questions (nothing blocking except the ropllup error so far)
If you can't solve the rollup error without having access to a Mac we can organize a call next week if you want

server/tests/e2e/federated.spec.ts

discojs/src/processing/image.ts

discojs/src/processing/index.ts

discojs/src/models/gpt/index.ts

discojs/src/validator.ts

cli/src/args.ts

webapp/cypress/support/e2e.ts

tharvik · 2024-10-04T18:34:39Z

You seem to be pushing typescript to its limits haha

héhé, yeah, it's what I do in pretty much any coding languages, pushing it to its limits then complaining about it (:

I couldn't review everything yet because of this but I already left a lot of questions

yeah,I think that the more we progress, the long the review will be, as we both know more of the code, what can get wrong, and are getting opiniated on various components. I feel quite good about it in fact, that means we care more for disco 🪩

If you can't solve the rollup error without having access to a Mac we can organize a call next week if you want

fwiw, it was indeed a mac specific issue, that gets only triggered there, and will probably happen again if there is an update of rollup that I do on my side. well, now we know

JulienVig

Amazing refactor!!

Mainly, I noticed that testing an image model always shows the images as red (while the accuracy is 100% in this case). That can be a simple UI mistake but maybe a deeper problem in the validation.
Not sure if this was introduced in this PR or the dark theme but when there is only one datapoint the chart now shows a bar instead of a dot
Also the x-axis labels are getting crowded
Testing with a language model shows a "Download CSV" button that doesn't work

discojs/src/types/data_format.ts

discojs/src/dataset/dataset.ts

discojs/src/models/tfjs.ts

server/src/task_set.ts

tharvik · 2024-10-28T11:53:47Z

Mainly, I noticed that testing an image model always shows the images as red (while the accuracy is 100% in this case). That can be a simple UI mistake but maybe a deeper problem in the validation.

ouf, it was a simple UI mistake, I inversed red & green in #682

Not sure if this was introduced in this PR or the dark theme but when there is only one datapoint the chart now shows a bar instead of a dot

Also the x-axis labels are getting crowded

ha yes, good point, I reworked it to make it a bit nicer. the curve is now "cubic" instead of "smooth" which makes it less jittery at the expense of precision.

Testing with a language model shows a "Download CSV" button that doesn't work

right, it is not implemented. I wasn't sure of what to show in it so I prefered to throw a TODO. now, do we want to even have this feature? I feel that it is not really useful (for testing that is, for prediction it is useful). we need a way to present testing results but CSV is too technical.

images: we already have a nice way to show the results
tabular: yes, not really nice but at least it is somewhat readable (colors would be nicer)
text: I can think of way to present it: for every input line (styled as gray), show the next word in green/red

WDYT? (for a next iteration)

tharvik force-pushed the 650-framework-agnostic-preprocessing-tharvik branch 3 times, most recently from a9d1035 to 0ba102d Compare September 24, 2024 10:25

tharvik marked this pull request as ready for review September 24, 2024 10:35

tharvik requested a review from JulienVig September 24, 2024 10:35

JulienVig mentioned this pull request Sep 26, 2024

Disco preprocessing is too constraining #649

Open

2 tasks

JulienVig requested changes Sep 26, 2024

View reviewed changes

tharvik force-pushed the 650-framework-agnostic-preprocessing-tharvik branch 3 times, most recently from 5323bed to 88b20b3 Compare September 30, 2024 14:45

tharvik mentioned this pull request Oct 2, 2024

Allow decentralized users to join late and catch up #775

Merged

4 tasks

tharvik force-pushed the 650-framework-agnostic-preprocessing-tharvik branch 3 times, most recently from 7d520b8 to 0ac6c29 Compare October 4, 2024 09:42

tharvik force-pushed the 650-framework-agnostic-preprocessing-tharvik branch from 9a611b7 to dbbf432 Compare October 4, 2024 18:46

tharvik requested review from JulienVig and removed request for JulienVig October 21, 2024 07:57

tharvik force-pushed the 650-framework-agnostic-preprocessing-tharvik branch 2 times, most recently from fe0e1c8 to cca6dc3 Compare October 23, 2024 12:43

tharvik requested a review from JulienVig October 23, 2024 12:55

tharvik mentioned this pull request Oct 23, 2024

Tabular columns are not checked anymore before training #801

Closed

JulienVig approved these changes Oct 24, 2024

View reviewed changes

tharvik added 7 commits October 25, 2024 21:50

server/tests: bump timeout

a512f5a

server/test/status: avoid arbitrary wait

0968620

discojs/processing: expand

c19b42c

discojs/dataset: allow subclass

8266afc

discojs/dataset: add cache

33e0c7c

discojs/dataset: resolve batch in parallel

76ea663

discojs/model: generic on datatype

7433b0b

tharvik added 22 commits October 28, 2024 10:12

discojs/disco: option to keep preprocessed

ce187ba

discojs/types: use better names

a52865b

discojs/validator: flatten

977b66f

discojs/processing: add validator specifics

35d09c5

discojs/types/text: single output token

e09b6ff

*: bump deps

654c4b6

discojs/model: generic predict

160f0e1

discojs/tabular: single output

2cb4864

discojs: rm old processing

b73d764

discojs/task: generic on datatype

f02b7e8

discojs: simplify types

707447f

webapp/tsconfig: add cypress

e457cd7

discojs/dataset/image: documente

32715f2

*: fix typo "excepted" -> "expected"

d16c283

discojs/dataset: clearer this binding

225bd9a

server/tasks: drop remote model loading

e9e46eb

discojs/model/tfjs: drop text support

815614e

discojs/dataset: add unbatch

2ec74df

discojs/validator: output Inferred

98a64c2

discojs/model/gpt: commonize predict

6a86f83

discojs/disco: document preprocessOnce

3f53cdb

discojs/types: add DataFormat namespace

4f0bc55

tharvik force-pushed the 650-framework-agnostic-preprocessing-tharvik branch from cca6dc3 to 4f0bc55 Compare October 28, 2024 09:12

tharvik added 2 commits October 28, 2024 10:44

webapp/testing: fix correct color

d4b1c25

webapp/training: nicer graphs

f680523

tharvik merged commit a5735e8 into develop Oct 28, 2024
23 checks passed

tharvik deleted the 650-framework-agnostic-preprocessing-tharvik branch October 28, 2024 12:00

tharvik mentioned this pull request Nov 1, 2024

Fix and rework GPT-TF.js #807

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: framework agnostic preprocessing #781

*: framework agnostic preprocessing #781

tharvik commented Sep 17, 2024

JulienVig left a comment •

edited

Loading

tharvik commented Oct 4, 2024

JulienVig left a comment

tharvik commented Oct 28, 2024 •

edited

Loading

*: framework agnostic preprocessing #781

*: framework agnostic preprocessing #781

Conversation

tharvik commented Sep 17, 2024

JulienVig left a comment • edited Loading

Choose a reason for hiding this comment

tharvik commented Oct 4, 2024

JulienVig left a comment

Choose a reason for hiding this comment

tharvik commented Oct 28, 2024 • edited Loading

JulienVig left a comment •

edited

Loading

tharvik commented Oct 28, 2024 •

edited

Loading