[Logiquo]

Sixth PR in the stack toward a unified sequence representation, on top of #1354. It adds two pooling baselines over sequence_v2 fields and static tensors, built on SequenceEmbeddingModel.

Changes

• LogisticRegression_V2 embeds each level and mean-pools the records of every group bottom-up (an empty group gives zeros). It concatenates the field summaries with the raw static vectors and applies one linear layer.
• MLP_V2 adds an MLP at every level and on each static field before pooling. hidden_dim, num_layers and dropout are set per field.
• Per-step labels: each label record uses the mean of the top-level records at or before its time (a prefix mean), so it never sees later records. Fields need a series top level.
• Both are exported from pyhealth.models and listed in the model guide, with API pages.

[github-actions]

Test Coverage

Total: 75.23% (21532 / 28623 lines)

Modified file	Coverage	Lines
pyhealth/models/__init__.py	100.00%	56 / 56
pyhealth/models/logistic_regression.py	86.00%	129 / 150
pyhealth/models/mlp.py	76.67%	115 / 150

Workflow run · 2e08ba8

[Logiquo]

/review

[github-actions]

_{Reviewed commit 2e08ba8}

[github-actions]

[medium] Per-step and bottom-up logic duplicates RNN_V2 instead of sharing it

The per-step branch (label-time padding, searchsorted over padded times, prefix indexing) and the bottom-up level loop repeat RNN_V2._at_labels and _FieldRNN._bottom_up in pyhealth/models/rnn.py. The init field classification and per-step series check also mirror RNN_V2. Only the per-level reducer differs (mean here, RNN there). Fix: move the shared level walk and label-time readout into helpers in rnn.py (or a common module) parameterized by the per-level encoder, and have LogisticRegression_V2 and RNN_V2 both use them. This blocks merge because it is a copy of existing logic, not a reuse.

[github-actions]

[low] Prefix mean subtracts batch-wide float32 cumsums

prefix is a cumsum over every row in the batch, and each sample's mean is read as a difference of two global prefix values. Rounding error therefore grows with total batch size and makes each sample's output depend slightly on the other samples. The comment at line 348 says the result is independent. Tests use small batches, so this is not caught. Fix: take per-group prefix sums (for example, cumsum within each group via the padded layout already built above, or index a padded cumsum), so each sample's result depends only on its own rows.

[github-actions]

[low] LogisticRegression_V2 is tested only inside the MLP_V2 test file

The file is named test_mlp_v2.py but also covers LogisticRegression_V2 throughout. Renaming it to a neutral name, or splitting out tests/core/test_logistic_regression_v2.py, would make the coverage easier to find. Not a merge blocker.

[github-actions]

AI Review

suggestion: need change
decision: need change

Thank you for this contribution. MLP_V2 and LogisticRegression_V2 add pooling baselines over sequence_v2 fields and static tensors. They include causal per-step prefix means, exports, API docs, and a thorough test file. The code looks correct as far as I can tell, and the tests cover hierarchy means, empty groups, masks, pretrained tokens, causality and Trainer runs.

To get this merged:

• Remove the duplicated logic. LogisticRegression_V2 copies RNN_V2's per-step label indexing and its bottom-up level walk. It also copies RNN_V2's constructor validation. The encoder is different (mean pooling instead of an RNN), but the shared parts should be factored into a common helper so the two models don't drift apart. Reusing _groups and _pool is a good start. Please extend that to the label-time indexing and level traversal.
• Raise the test coverage of pyhealth/models/mlp.py. It is at 76.67%, below the 80% bar for modified files. Add tests that cover the uncovered lines. Most of those lines are probably in the existing MLP class. A few tests for the paths you touched, or for the neighboring code, should be enough to reach 80%.

Whole-repo coverage is 75.23%, above the 60% bar. logistic_regression.py is at 86% and __init__.py at 100%.

Optional suggestions:

• The prefix mean subtracts batch-wide float32 cumulative sums. That adds small rounding error that depends on batch composition. Computing the sums per sample, or in float64, would avoid this.
• Consider adding an example script for each model.
• Consider moving the LogisticRegression_V2 tests into their own test file, since they currently live in tests/core/test_mlp_v2.py.

_{Reviewed commit 2e08ba8}