Skip to content
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.

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

burn-train in the browser #921

Open
AlexErrant opened this issue Nov 1, 2023 · 1 comment
Open

burn-train in the browser #921

AlexErrant opened this issue Nov 1, 2023 · 1 comment
Labels
feature The feature request

Comments

@AlexErrant
Copy link
Contributor

Feature description

I'd like to run burn-train in the browser. ndarray and the train-minimal feature is sufficient.

Feature motivation

I'm building https://github.com/AlexErrant/fsrs-wasm/ which is basically https://github.com/open-spaced-repetition/fsrs-rs/ ported to the browser. In particular, I'm interested in training FSRS.

For now, performance is not a goal. I may revisit this later if it proves to be an issue. Right now my dataset is on the order of ~100k FSRSReviews, which is two u32s. (We'll see if ndarray is enough to train on that in a reasonable amount of time - if not I may have a second go at wgpu.)

Suggest a Solution

To be clear, I am interested in implementing this. However, this would be my first non-trivial contribution to this project, and I'm seeking feedback on architecting this feature.

Would burn accept a PR adding browser support to burn-train? I'm currently interested in augmenting burn-train, not creating a new crate. I'll be adding a new feature, browser, that will switch between different implementations of invokers of std::thread::spawn, std::fs, and possibly others. These may include (but are not limited to) FileCheckpointer, EventStoreClient, AsyncCheckpointer, AsyncLogger, and Worker.

Where possible, I will continue using std; targeting no_std is not a goal. My understanding is that for targeting the browser, no_std is sufficient, but not necessary. For example, this blog states that std::sync::mpsc may be used for communicating between web workers.

Preventing the code from becoming virally async is a goal. Asyncify may be used as a last resort.

As a first step, I plan on converting EventStoreClient to conditionally use a web worker, upon which I'll open a draft PR. I plan on trying out the above blog's spawn implementation. I'm not 100% on it since... well frankly the example code doesn't compile... but I thought I should open the issue and ensure that my goals align with this project's before I go too deep.

@nathanielsimard
Copy link
Member

I believe it's a valuable feature if we aim to minimize the use of 'async' wherever possible, even in the browser. So, if you believe a solution can be developed without relying on async calls, then please pursue it, starting with a reasonably small pull request.

@louisfd louisfd added the feature The feature request label Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature The feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants