Skip to content

Model registry derives wrong algorithm names via str.lstrip("m_") #68

Description

@thomasATbayer

Model registry derives wrong algorithm names via str.lstrip("m_")

Summary

MotherModelRegistry._load_models() derives an algorithm name from each
m_*.py model module filename using str.lstrip("m_"). str.lstrip removes a
set of leading characters, not a prefix, so any model file whose name starts
with a character that is also present in "m_" gets silently mangled.

The most visible case: a module named m_mlp.py produces the algorithm name
"lp" instead of "mlp", because lstrip strips the leading m, then _,
then the second m as well, stopping only at l.

Location

src/mother/ml/__init__.py, in MotherModelRegistry._load_models():

algo: str = model_file.lower().lstrip("m_")

Why it is wrong

str.lstrip(chars) treats its argument as a set of characters to strip from the
left, repeatedly, until a character not in the set is found. It is not a
prefix removal.

>>> "m_mlp".lstrip("m_")
'lp'          # expected 'mlp'
>>> "m_catboost".lstrip("m_")
'catboost'    # happens to be correct (next char 'c' is not in {'m','_'})
>>> "m_flow".lstrip("m_")
'flow'        # correct by luck

Any module name of the form m_m... (a model family whose name starts with m,
e.g. mlp, mars, mlr) is corrupted. Names that don't start with m after
the prefix are corrupted only if their leading characters fall in {"m", "_"},
which is why the bug went unnoticed for the existing m_catboost, m_lasso,
m_node, m_randomforest, m_tabpfn modules.

Impact

  • get_available_algorithms() returns incorrect algorithm tokens (e.g. lp).
  • get_model_class_by_algorithm_and_type(algorithm, model_type) cannot resolve
    the mangled name and raises
    ValueError: Unsupported algorithm '<name>' or model type '<type>'.
  • Any config-driven or test code that sweeps over get_available_algorithms()
    breaks for the affected model. This surfaced as failures in
    test/unit/test_ml.py once an m_mlp.py module was added.

Reproduction

from mother import ml

# Add any model module named m_mlp.py under src/mother/ml/models/
# containing an AbstractMotherPipeline subclass, then:
print(ml.get_available_algorithms())        # -> [..., 'lp']  (should be 'mlp')
ml.get_model_class_by_algorithm_and_type("mlp", "regression")  # KeyError/ValueError

Fix

Use str.removeprefix (Python 3.9+), which removes the literal prefix once:

algo: str = model_file.lower().removeprefix("m_")
>>> "m_mlp".removeprefix("m_")
'mlp'
>>> "m_catboost".removeprefix("m_")
'catboost'

Suggested follow-ups

  • Add a regression test asserting that a module named m_mlp maps to the
    algorithm "mlp" (and generally that algo == model_file with the m_
    prefix removed).
  • Audit for other misuses of str.lstrip/str.rstrip where a prefix/suffix
    removal was intended.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions