-
Notifications
You must be signed in to change notification settings - Fork 2
Adds batching and CTCLoss functionality to RNNProcessor #16
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
base: dev
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -35,6 +35,12 @@ class RNNSettings(TorchModelSettings): | |
| If False, the hidden state will be reset at the start of each window. | ||
| If "auto", preserve if there is no overlap in time windows, otherwise reset. | ||
| """ | ||
| batch_train: bool = False | ||
| """ | ||
| When True, train on the full batch in a single forward/backward pass | ||
| using packed sequences to handle varying sequence lengths. | ||
| When False, train each sample individually (current behavior). | ||
| """ | ||
|
|
||
|
|
||
| class RNNState(TorchModelState): | ||
|
|
@@ -154,8 +160,10 @@ def _train_step( | |
| X: torch.Tensor, | ||
| y_targ: dict[str, torch.Tensor], | ||
| loss_fns: dict[str, torch.nn.Module], | ||
| input_lens: torch.Tensor | None = None, | ||
| target_lens: torch.Tensor | None = None, | ||
| ) -> None: | ||
| y_pred, self._state.hx = self._state.model(X, hx=self._state.hx) | ||
| y_pred, self._state.hx = self._state.model(X, hx=self._state.hx, input_lens=input_lens) | ||
|
Contributor
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. This will break existing custom RNN models that can be used with the RNN processor if they don't accept input_len argument in their forward pass. If we want to enforce that the models have the input_lens parameter, we can do that (I think its would just break a couple of my models, so I could update those), otherwise it might be good only pass the kwarg when it's actually set. |
||
| if not isinstance(y_pred, dict): | ||
| y_pred = {"output": y_pred} | ||
|
|
||
|
|
@@ -167,6 +175,12 @@ def _train_step( | |
| raise ValueError(f"Loss function for key '{key}' is not defined.") | ||
| if isinstance(loss_fn, torch.nn.CrossEntropyLoss): | ||
| loss = loss_fn(y_pred[key].permute(0, 2, 1), y_targ[key].long()) | ||
| elif isinstance(loss_fn, torch.nn.CTCLoss): | ||
| if input_lens is None or target_lens is None: | ||
| raise ValueError("CTCLoss requires input_lens and target_lens in batch training mode.") | ||
| log_probs = torch.nn.functional.log_softmax(y_pred[key], dim=-1).permute(1, 0, 2) | ||
| targets = y_targ[key].flatten().long() | ||
|
Contributor
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.
From the |
||
| loss = loss_fn(log_probs, targets, input_lengths=input_lens, target_lengths=target_lens) | ||
| else: | ||
| loss = loss_fn(y_pred[key], y_targ[key]) | ||
| weight = loss_weights.get(key, 1.0) | ||
|
|
@@ -210,7 +224,23 @@ def partial_fit(self, message: AxisArray) -> None: | |
| loss_fns = {k: loss_fns for k in y_targ.keys()} | ||
|
|
||
| with torch.set_grad_enabled(True): | ||
| if preserve_state: | ||
| if self.settings.batch_train: | ||
| input_lens = message.attrs.get("data_len") | ||
| target_lens = message.attrs.get("trigger_len") | ||
| if input_lens is not None: | ||
| input_lens = torch.tensor(input_lens, dtype=torch.int64, device="cpu") | ||
| if target_lens is not None: | ||
| target_lens = torch.tensor(target_lens, dtype=torch.long, device=self._state.device) | ||
| self.reset_hidden(batch_size) | ||
| self._train_step(X, y_targ, loss_fns, input_lens=input_lens, target_lens=target_lens) | ||
| else: | ||
| ez.logger.warning( | ||
|
Contributor
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. If loss is CTC, then fall-back will error since it needs data_len. Consider if we should give an informative ezmsg logger error here rather than wait for it to hit the train step and error there. |
||
| "batch_train=True but 'data_len' not in message.attrs; falling back to per-sample training." | ||
| ) | ||
| self.reset_hidden(batch_size) | ||
| for i in range(batch_size): | ||
| self._train_step(X[i].unsqueeze(0), {k: v[i].unsqueeze(0) for k, v in y_targ.items()}, loss_fns) | ||
|
Comment on lines
+241
to
+242
Contributor
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. This fall-back is for batch_train=True but no 'data_len'. Should this not still be a single train step for the batch rather than loop over the batch? Maybe this whole section can turn into something like this? |
||
| elif preserve_state: | ||
| self._train_step(X, y_targ, loss_fns) | ||
| else: | ||
| for i in range(batch_size): | ||
|
|
||
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.
Might be good to specify that when this is true, the partial fit message must contain data_len and trigger_len.