lucawrabetz / opti-face

A suite of tools for automating experiments in computational research.
MIT License
9 stars 1 forks source link

Transition the Feature class to an appropriate pydantic supported object. #25

Open lucawrabetz opened 3 weeks ago

lucawrabetz commented 3 weeks ago

Overview

Use a pydantic object to define Feature.

Description

The Feature class is the crux of current opti-face MVPs, it allows us as the implementer to declare a set of Features, representing all the columns in the results database, that have a strict typing enforced, validation rules (depends on the mvp), and consistent names for ui logging. By adding features to a global list of declared features, the implementer can handle database migration (this was easy to handle in a hacky way in the csv database system), and ensure that the experiments library had a consistent set of instance parameters, solver parameters, and outputs (the three Feature categories to work with).

Migrating this core component to a pydantic base class seems like a no brainer - pydantic provides data validation and functionality for "defaults", in addition to being touted as an excellent library for integrations with other dependencies in opti-face (SQLModel, etc.). The key question, (as a newbie to pydantic) is exactly how to do this:

Possible solutions or directions

  1. Feature is a BaseModel
  2. The entire set of Features (all columns in a row) is a BaseModel, as the Field functionality in pydantic seems to cover everything we want to implement with Feature.

Starting with (1) in our current commits, but let's use this issue as a forum to come to a decision on this.

P.S. I wrote all this assuming we fully decouple the Feature component used by optiface components, from whatever similar Feature classes may be implemented on the db side (such as the Config classes in #24), but this is also up for discussion if it is overkill.

lucawrabetz commented 2 weeks ago

Important note to clarify: SQLModel is a BaseModel, so TableConfigCreate.create_dynamic_model from Peter's PR (#24) returns a BaseModel (it returns a SQLModel).

This "config" is the feature set we are building off of on the db side, which is generated by reading a JSON file. I will finish my PR #23 with refactoring the feature set to json, then if we can finish both PRs with the db-side TableConfigCreate.create_dynamic_model() reading from the same feature json file as the core interface global feature state reads, we should be in good shape.

This comment means we kind of addressed the decoupling question in our PRs already and I think it's a no-brainer to keep it that way (decoupled feature sets to work with on the core interface and db side) as you don't really want the core interface passing around a SQLModel object as the feature set when it is not doing any DB work.

The question about whether the feature set on that side can (and / or should) be one BaseModel with the features as fields, or whether each feature should be a BaseModel is still open, I guess.

@Peter-Sanders @alvinhenrick let me know if there is some cleaner way to do this or if you have comments.