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: Initial Support for FlatBuffer context #608

Draft
wants to merge 25 commits into
base: master
Choose a base branch
from

Conversation

lokitoth
Copy link
Member

@lokitoth lokitoth commented Feb 14, 2024

Add support for using flatbuffer spans as input to RLClientLib when using a VW model.

Note: Currently only working in CB models when using choose_rank().

  • Fix memory leak when providing bad buffer (e.g. JSON) to a Flatbuffer-configured LiveModel
  • Figure out why the build breaking on fb_parser_test
    • reinforcement_learning builds on ubuntu1804; vowpal_wabbit builds on ubuntu2004
  • Verify whether CA models work out of the box with Flatbuffer contexts
  • When no more changes to VW need to be made, return submodule back to pointing to a trunk commit of VW

Stretch:

  • Support RequestDecision/MultislotDecision (need to implement context introspection for FB contexts)
  • (unlikely) Support Episodic (need to implement history generation / merge; to do this right requires a fair bit of engineering)

Currently only working in CB models when using choose_rank().
* Also implement a mechanism to manage collisions between RL and VW api_status.h
Two issues causing a build break due to mismatch between dev and ci C++ standards version:

* Earlier C++ standards had problems with detecting types in lambdas for auto
* "default" keyword could not be used as a variable name
* Compiler cannot disambiguate between typename and variable shadowing the typename in contexts requiring typename
Testing to see if this is causing a build-break in the build nugets workflow
Older versions of the C++ standard could not properly deduce the chain of types to be built up using initializer lists.
There is a fair bit of logic in RLClientLib which is very payload-specific, particularly around dedup and the more complex loop types.

In the initial support for FlatBuffer contexts we are going to avoid enabling those scenarios to get to E2E FlatBuffer CB, so we suppress these capabilities when we use FlatBuffer.

The fix is to do this correctly and in all cases so we do not run into weird AccessViolation exceptions / SegFaults.

The longer-term fix would be to provide a higher-level way to interact with the context (likely via the underlying i_model) to avoid having to deeply understand every single input_serialization type. This is a fairly significant refactor, and beyond the scope of the current effort around FlatBuffers.
Also deprecate onnx.use_unstructured_input in favour of the INPUT_SERIALIZATION configuration parameter.
* Adds support for fb inputs for CB loops
* Adds helper for generating fb inputs
* Cleans up ActionFlags APIs a little
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.

1 participant