Repository navigation
RFC: Refactor the ModelManager #3545
bitgamma
started this conversation in
Request for Comment (RFC)
Replies: 1 comment 1 reply
|
Thanks for starting this discussion! I need to read and understand it in depth another time, but we should absolutely not have 6.7k LoC files so clearly something should be done. Something I've discussed with @ramkrishna2910 is that there seems to be some smart-router-specific functionality burried deep in ModelManager as well. Curious to get your take @bitgamma on whether that should come out as well, as I don't see the smart router mentioned in this RFC yet. |
1 reply
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Proposal size
Major feature (spans multiple well-scoped PRs)
Updates
No response
User story
The goal is to restructure the model manager to be able to easily add data sources, catalogs and storage options. Another goal is to split this big components into more manageable and hopefully easier to unit-test subcomponents.
High-level design
ModelManager does a lot of stuff and is one of the core components of lemonade. The things it handles are:
All these concerns are inherently related but currently there is no clear separation, which result in model_manager.cpp being over 6.7k LoC.
My proposal is to refactor this into a modular system, where each concern is handled through interfaces. Let's call them:
There will be a coordination layer to wire these layers and the model definitions must be enriched with the download source (possibly listing alternative sources for the same file) to make this wiring possible.
The ModelIndex keeps track of which models are actually available for download and running, with a filtering layer (following the current filtering rules) and an aliasing layer (which relies on the existing AliasManager. I am not sure if the indexer should run completely at startup (like it does now) or if should have a persistent index to make things easier.
Breaking changes
A migration will be needed for the data structures and current model-related json files. The model manager API will likely somewhat change, but the goal is to restructure its internal API rather than what it exposes
Maintenance plan
this being a core component it will be inevitably touched many times. Hopefully the ability to test components in isolation should make the code more robust.
Risks
there is a lot that could go wrong during implementation and very careful testing will be needed. It is also likely we will observe some regressions that will need to be addressed
All reactions