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

feat: WASM initial CB model API #4574

Merged
merged 7 commits into from
Apr 26, 2023
Merged

Conversation

olgavrou
Copy link
Collaborator

@olgavrou olgavrou commented Apr 26, 2023

For the cb model, the example is not opaque but a javasctipt object that the C++ side can inspect

Moved basic vw functionality out into vw_model_basic class which is used as a mixin for the vw_model which is generic vw model implementation and the newly added cb_vw_model

vw_model_basic can be enhanced in the future with any other generic functionality

wasm/wasm_wrapper.cc Outdated Show resolved Hide resolved
@@ -40,6 +40,9 @@ void validate_options(const VW::config::options_i& options)
if (!options.get_positional_tokens().empty()) { THROW("Positional options are not allowed") }
}

struct vw_model_basic;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove Line 43 - 45 and set the default MinxIn on Line183? to be consistent with cb_vw_model?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these need to be forward declared so that vw_model can be referenced in line 55

@olgavrou olgavrou merged commit 795cd75 into VowpalWabbit:master Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants