Repository navigation
Add timeseries_binary and timeseries_regression model modes #1336
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -225,6 +225,13 @@ def __init__( | |
| raise ValueError("hidden_size is determined by hidden_dim") | ||
| assert len(self.label_keys) == 1, "Only one label key is supported if RNN is initialized" | ||
| self.label_key = self.label_keys[0] | ||
| # Per-step (timeseries) labels predict at every input step. | ||
| self._per_step = self.mode in ("timeseries_binary", "timeseries_regression") | ||
| if self._per_step and kwargs.get("bidirectional", False): | ||
| raise ValueError( | ||
| "A per-step label needs a causal model; bidirectional=True would " | ||
| "use future time steps." | ||
| ) | ||
|
|
||
| self.embedding_model = EmbeddingModel(dataset, embedding_dim) | ||
|
|
||
|
|
@@ -302,14 +309,30 @@ def forward(self, **kwargs) -> Dict[str, torch.Tensor]: | |
| else: | ||
| mask = (torch.abs(x).sum(dim=-1) != 0).int() | ||
|
|
||
| _, x = self.rnn[feature_key](x, mask) | ||
| patient_emb.append(x) | ||
|
|
||
| patient_emb = torch.cat(patient_emb, dim=1) | ||
| # (patient, label_size) | ||
| outputs, x = self.rnn[feature_key](x, mask) | ||
| # Per-step labels use the output at every step, else the last one. | ||
| patient_emb.append(outputs if self._per_step else x) | ||
|
|
||
| if self._per_step: | ||
| # Features without a time axis (one step) are repeated over time. | ||
| steps = max(emb.shape[1] for emb in patient_emb) | ||
| for i, emb in enumerate(patient_emb): | ||
| if emb.shape[1] == 1: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] Single-step time series are silently broadcast like static features The per-step branch repeats any feature whose batch time length is 1 across all steps. A genuine time series that has a single step in this batch is indistinguishable from a static tensor, so a grid mismatch with a longer series passes without the intended 'same time grid' ValueError. Fix: decide which features are static from the input path (the |
||
| patient_emb[i] = emb.expand(-1, steps, -1) | ||
| elif emb.shape[1] != steps: | ||
| raise ValueError( | ||
| "Per-step labels need all time series features on the " | ||
| "same time grid." | ||
| ) | ||
| patient_emb = torch.cat(patient_emb, dim=-1) | ||
| # (patient, label_size), or (patient, steps, label_size) per step | ||
| logits = self.fc(patient_emb) | ||
| # obtain y_true, loss, y_prob | ||
| y_true = kwargs[self.label_key].to(self.device) | ||
| if self._per_step: | ||
| # (value, mask) per step; the loss averages over labeled steps. | ||
| y_true = tuple(t.to(self.device) for t in kwargs[self.label_key]) | ||
| else: | ||
| y_true = kwargs[self.label_key].to(self.device) | ||
| loss = self.get_loss_function()(logits, y_true) | ||
| y_prob = self.prepare_y_prob(logits) | ||
| results = {"loss": loss, "y_prob": y_prob, "y_true": y_true, "logit": logits} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[medium] Per-step modes are accepted by every model but only RNN implements them
VALID_MODESandget_loss_functionnow resolvetimeseries_binary/timeseries_regressionfor anyBaseModelsubclass. OnlyRNN.forwardproduces per-step logits and tuple labels. Other models (e.g. MLP, Transformer, TCN, which callself.get_loss_function()(logits, y_true)with a tuple label) fail at loss time with an AttributeError (.to()on a tuple) or an unpacking/IndexError inside_masked_step_loss, instead of a clear message. Fix: reject per-step modes in models that do not implement them, for example inBaseModel.moderesolution with an opt-in class flag that only RNN sets, and add a test that another model raises a clear ValueError. Alternatively document RNN as the only supported model.