Superseded by #85: Investigate HMM backend surface - #79
Conversation
| """Configuration of a single trainable HMM submodule.""" | ||
|
|
||
| name: str | ||
| role: Literal["transition", "stride", "other"] |
There was a problem hiding this comment.
why "other" should be Literal["transition", "stride"] | str
|
|
||
| def __init__( | ||
| self, | ||
| modules: dict[str, HmmSubModelConfig] = cf(_default_modules()), |
There was a problem hiding this comment.
Dicts are bad for tpcp. Check the docs list of tuples is better
| To rephrase it again: We want to create a fully labeled dataset with already optimal hidden-state labels, but as | ||
| these lables are hidden, we need to predict them with our already trained models... | ||
| """ | ||
| """Create fully labeled gait sequences from typed regions and trained submodels.""" |
There was a problem hiding this comment.
Expand doccstring with more info again.
| Sequence of gaitmap stride lists. | ||
| The number of stride lists must match the number of sensordata objects (i.e. they must belong together). | ||
| region_list_sequence | ||
| Sequence of typed region lists. |
There was a problem hiding this comment.
Explain the expected format
| architecture="left-right-strict", | ||
| name="stride_model", | ||
| @classmethod | ||
| def _from_json_dict(cls, json_dict: dict) -> Self: |
There was a problem hiding this comment.
This is done for backwards compatibility right? Add a todo note that we should remove this again and rather convert the old model to the correct format
| return sum(module.n_states for module in self.model_config.modules.values()) | ||
|
|
||
| @property | ||
| def module_offsets(self) -> dict[str, int]: |
There was a problem hiding this comment.
Not relevant for user facing -> _module_offsets
|
|
||
| # predict hidden state labels for complete walking bouts | ||
| module_offsets = self.module_offsets | ||
| labels_train_sequence = create_fully_labeled_gait_sequences( |
There was a problem hiding this comment.
Should we rename create_fully_labeled_gait_sequences?
| return normalized_region_list | ||
|
|
||
|
|
||
| def convert_region_list_to_transition_list( |
There was a problem hiding this comment.
Can we find a more performant version for this? I think we have a couple of helper funcs in utisl that can help here.
|
|
||
| def _stride_list_to_region_list(stride_list: pd.DataFrame, region_type: str = "stride") -> pd.DataFrame: | ||
| region_list = stride_list[["start", "end"]].copy() | ||
| region_list.insert(0, "roi_id", np.arange(len(region_list))) |
There was a problem hiding this comment.
Just rename stride_id (the index to roi id)
| return log_emissions | ||
|
|
||
|
|
||
| def _viterbi_decode(model: HMMState, log_emissions: np.ndarray) -> np.ndarray: |
There was a problem hiding this comment.
Can we use numba to speed this and other funcs up here?
| return np.argmax(posterior, axis=1) | ||
|
|
||
|
|
||
| class ScipyHmmInferenceBackend(BaseHmmBackend): |
There was a problem hiding this comment.
Move the scipy backend in its own file, so that we can import it even when pomegrante isnot installed
| region_list_sequence = [] | ||
| for sensor in ["left_sensor", "right_sensor"]: | ||
| region_list = stride_list[sensor][["start", "end"]].copy() | ||
| region_list.insert(0, "roi_id", np.arange(len(region_list))) |
There was a problem hiding this comment.
use assign and reuse the stride index/stride id
| return tuple(transitions) | ||
|
|
||
|
|
||
| class PomegranateHmmBackend(BaseHmmBackend): |
There was a problem hiding this comment.
Move to dedicated file to allow imports of base and scipy backend when pomegrante not installed
| model = fix_model_names(model) | ||
| model.bake() | ||
| model.freeze_distributions() | ||
| return _clone_model(model, assert_correct=False) |
There was a problem hiding this comment.
Do we need to clone, given that we just created the model here?
| return tuple(module.name for module in self.modules if module.role == "stride") | ||
|
|
||
| @property | ||
| def custom_model_names(self) -> tuple[str, ...]: |
| model_config: CompositeHmmConfig = cf(CompositeHmmConfig()), | ||
| feature_transform: RothHmmFeatureTransformer = cf(RothHmmFeatureTransformer()), | ||
| *, | ||
| algo_predict: Literal["viterbi", "map"] = "viterbi", |
There was a problem hiding this comment.
Wouldn't it make more sense to have the training params on teh composite config instead of here?
|
|
||
|
|
||
| class RothHmmConfig(_BaseSerializable): | ||
| """Serializable configuration bundle for `RothSegmentationHmm`.""" |
There was a problem hiding this comment.
Docstring should explain all parameters (Same for all other classes in teh config module)
| self.backend = backend | ||
|
|
||
| @property | ||
| def model_config(self) -> CompositeHmmConfig: |
There was a problem hiding this comment.
Remove all the roperty mappings. user should just access everything through hmm_config
| return model | ||
|
|
||
|
|
||
| def pomegranate_model_to_hmm_state( |
There was a problem hiding this comment.
move to pomegrante_legacy_backend file
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #79 +/- ##
==========================================
+ Coverage 89.93% 89.95% +0.01%
==========================================
Files 61 61
Lines 3438 3424 -14
==========================================
- Hits 3092 3080 -12
+ Misses 346 344 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…igate/hmm-backend-surface
|
Superseded by #85, which implements the backend-neutral HMM model, swappable inference/training backends, reusable topology composition, and legacy JSON loading. Closing this earlier approach in favor of the completed replacement. |
No description provided.