Skip to content

Code Review: automated questions #64

@bes-dev

Description

@bes-dev

Code Review Questions

The following questions and observations were raised during automated review of bes-dev/MobileStyleGAN.pytorch.

  1. In train.py (~line 46), raise "Unknown export format." raises a plain string, not an exception — this is a TypeError in Python 3 and silently swallows the intended error message. Should be raise ValueError(...).
  2. In core/models/synthesis_network.py SynthesisNetwork.forward, _style = style[..., 3*i+1:3*i+4, :] is computed but m(hidden, style, _noise) passes the full style tensor — the per-layer slice is never used, breaking W+ style mixing in the teacher network.
  3. In core/models/utils.py NoiseManager.__init__, the guard if not None in noise checks whether any element is None and skips the entire list if so — likely intended to be a per-element check (if noise[i] is not None), causing all cached noise to be silently dropped whenever one entry is None.
  4. In core/distiller.py validation_step, self.student(style, noise=gt["noise"]) is called twice — the first result is discarded and the student is run again for loss. One forward pass is redundant and doubles inference cost during validation.
  5. In core/distiller.py validation_epoch_end, KID is computed by concatenating local-device outputs only with a # TODO: add all_gather for distributed mode comment. With multi-GPU DDP training the metric is incorrect and the TODO has no associated tracking issue.
  6. In core/distiller.py, teacher networks mapping_net and synthesis_net are only set to .eval() but never have requires_grad_(False) applied. Gradients still flow into teacher parameters during student training, wasting memory and compute.
  7. In core/distiller.py compute_mean_style, the batch_size parameter is accepted but ignored — torch.randn(4096, ...) is hardcoded. The method always uses 4096 samples regardless of the argument.
  8. In core/loss/distiller_loss.py loss_g, gt["img"] and gt["freq"] tensors from the teacher are not detached before computing L1/L2/perceptual losses, potentially letting gradients flow into the frozen teacher synthesis network.
  9. In core/models/modules/modulated_conv2d.py, ModulatedConv2d.get_demodulation uses self.style_inv (a randomly-initialised buffer, never updated) instead of the actual style vector. Demodulation is therefore style-independent, diverging from the StyleGAN2 weight-demodulation spec.
  10. In core/model_zoo.py, json.load(open(zoo_path)) opens a file handle that is never closed. Should use with open(zoo_path) as f: json.load(f) to avoid resource leaks.
  11. In core/models/modules/idwt_upsample.py, the class is named IDWTUpsaplme (typo — letters transposed). It is referenced by this name in mobile_synthesis_block.py, making refactoring error-prone.
  12. In core/loss/non_saturating_gan_loss.py loss_d, both fake.detach() and real.detach() are called even though fake is already detached by the caller in Distiller.discriminator_step. The redundant .detach() on real could silently prevent R1 gradient computation if reg_d is mistakenly called through loss_d.
  13. In core/distiller.py make_sample, the else branch (pure noise path) constructs style = self.mapping_net(var).view(1, self.wsize, ...) with batch-size 1 unconditionally, regardless of self.cfg.batch_size, so training occasionally runs with mismatched batch sizes without error.
  14. In core/models/modules/noise_injection.py NoiseInjection.forward, the noise buffer is registered on first call only (if not hasattr(self, "noise") and self.trace_model). Subsequent CoreML/ONNX traces reuse stale noise, but the buffer is never reset between exports, risking silent reuse of wrong-shape noise across multiple export calls.
  15. In requirements.txt, pytorch-lightning==1.0.2 is pinned to a 2020 release. ModelCheckpoint's filepath kwarg was deprecated and removed in later PL versions; the pinned version also predates security fixes. There is no upper bound or lock file to prevent breakage from transitive dependency updates.

Generated by automated code review pipeline.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions