Skip to content

Fix type annotations, dead code, VAE objective, and device consistency bugs from PR review#55

Merged
kellrott merged 2 commits into
developfrom
copilot/sub-pr-48
Mar 10, 2026
Merged

Fix type annotations, dead code, VAE objective, and device consistency bugs from PR review#55
kellrott merged 2 commits into
developfrom
copilot/sub-pr-48

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 10, 2026

Seven correctness and API hygiene issues identified in code review on PR #48, spanning type annotations, unreachable guards, a VAE objective bug, and device consistency gaps.

Changes

embkit/modules/__init__.py

  • Removed stale LayerInfo reference from module docstring; the type now lives at embkit.factory.layers.Layer

embkit/factory/mapping.py

  • Fixed get_activation() return type: Optional[nn.Module]Optional[Type[nn.Module]] — the function returns a class, callers instantiate it via get_activation("relu")()

embkit/models/vae/encoder.py

  • Dead code: Removed two unreachable if self.latent_dim is None: branches — latent_dim is a required int, these could never fire
  • VAE objective bug: Non-sampling forward path returned (mu, logvar, h) where h is the pre-head hidden state, meaning the decoder operated on a different tensor than the KL term. Fixed to return (mu, logvar, mu) so the deterministic latent is consistent end-to-end

embkit/models/vae/base_vae.py

  • BaseVAE.to() was calling .to() directly on encoder/decoder (crashing when either is None, skipping all other submodules, and not returning self). Replaced with return super().to(device=device, dtype=dtype) — PyTorch's submodule traversal handles everything correctly

embkit/factory/layers.py

  • Layer.gen_layer() now passes device/dtype to nn.BatchNorm1d, preventing mixed-device modules when a caller specifies a non-CPU device at build time

embkit/preprocessing/normalize.py

  • get_dataset_nonzero_mask() return type corrected from torch.Tensor to List[torch.Tensor]
  • Accumulator changed from torch.zeros(len(f)) (always CPU, float32) to torch.zeros(f.shape, dtype=torch.int32, device=f.device) to match the input tensor's device

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

…ice, encoder return value

Co-authored-by: kellrott <113868+kellrott@users.noreply.github.com>
Copilot AI changed the title [WIP] Collect changes for next release v0.3 Fix type annotations, dead code, VAE objective, and device consistency bugs from PR review Mar 10, 2026
@github-actions
Copy link
Copy Markdown

☂️ Python Coverage

current status: ❌

Overall Coverage

Lines Covered Coverage Threshold Status
2600 1235 48% 30% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
src/embkit/factory/layers.py 48% 🔴
src/embkit/factory/mapping.py 64% 🔴
src/embkit/models/vae/base_vae.py 67% 🔴
src/embkit/models/vae/encoder.py 89% 🔴
src/embkit/modules/_init_.py 100% 🟢
src/embkit/preprocessing/normalize.py 58% 🔴
TOTAL 71% 🔴

updated for commit: 5421f43 by action🐍

@kellrott kellrott marked this pull request as ready for review March 10, 2026 23:03
@kellrott kellrott merged commit 57233ae into develop Mar 10, 2026
1 check failed
@kellrott kellrott deleted the copilot/sub-pr-48 branch March 29, 2026 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants