Code Review: questions and findings

[bes-dev]

Code Review: Questions and Findings

1. train.py (line 44): raise "Unknown export format." raises a plain string, which is not valid in Python 3 — Python will raise TypeError: exceptions must derive from BaseException instead of the intended error message.

   The intended error is silently replaced by a confusing TypeError, masking the actual failure mode when an unsupported export format is supplied.

2. core/models/synthesis_network.py (line 73): Inside SynthesisNetwork.forward, _style = style if style.ndim == 2 else style[:, 3*i+1:3*i+4, :] correctly slices the per-block style, but the very next line passes the un-sliced style instead of _style to m(hidden, style, _noise). The computed slice is never used.

   Every synthesis block receives the full style tensor instead of its designated slice, making per-layer style modulation completely ineffective and producing incorrect outputs.

3. core/models/modules/ops/fused_act.py (line 30): raise NotImplemented raises the built-in constant NotImplemented (not an exception class), causing Python to throw TypeError: exceptions must derive from BaseException instead of the intended NotImplementedError.

   When neither CPU nor CUDA is available, the error raised is a confusing TypeError rather than a descriptive NotImplementedError, making debugging needlessly difficult.

4. core/models/modules/ops/upfirdn2d.py (line 13): Same raise NotImplemented bug as in fused_act.py: raises the built-in constant instead of NotImplementedError().

   Produces a misleading TypeError rather than a clear NotImplementedError when neither CPU nor CUDA path is available.

5. evaluate_fid.py (line 115): batch /= 255.0 is applied to a tensor that is already normalized to [0, 1] by TF.ToTensor() in ImagePathDataset. The same division also occurs in the BatchNorm fitting loop (line 98). This makes all pixel values ~255× too small (≈[0, 0.004]), far outside the expected range for the Inception model.

   TF.ToTensor() converts PIL uint8 images to [0.0, 1.0] floats; a subsequent /255 shrinks values to near zero, causing the Inception network to receive near-zero activations and producing completely invalid FID scores.

6. core/model_zoo.py (line 4): json.load(open(zoo_path)) opens a file handle that is never explicitly closed; if an exception is raised during JSON parsing the handle leaks.

   File handle leaks can exhaust OS file-descriptor limits, especially in long training runs that call model_zoo repeatedly.

7. core/distiller.py (line 57): compute_mean_style accepts a batch_size parameter but the implementation always uses the hardcoded literal 4096 (torch.randn(4096, ...)). The parameter is never referenced.

   The dead parameter misleads callers into believing they can control computation cost; the actual memory and compute usage is always fixed at 4096 samples regardless of the argument.

8. core/distiller.py (line 84): In validation_step, self.student(style, noise=gt["noise"]) is called twice: once to get pred_inc for KID evaluation and once to compute the validation loss. The two forward passes are redundant and inconsistent (the loss is computed on a second, independent call rather than reusing the first result).

   Doubling the student forward pass doubles GPU memory and compute time per validation step; using separate forward-pass outputs for metric and loss computation is also semantically inconsistent.

9. convert_rosinality_ckpt.py (line 46): channels.append(c_out) is executed after the while loop, but c_out is only assigned inside the loop body. If the loop breaks on the very first iteration (no matching weights found), c_out is undefined and a NameError is raised.

   A missing or misformatted checkpoint causes an unhandled NameError instead of a clear, descriptive error message about the absence of synthesis blocks.

10. core/utils.py (line 11): tensor_to_img performs in-place operations (clamp_, add_) on the input tensor t when normalize=True. Callers that retain a reference to the original tensor will see it silently mutated.

Unintended in-place modification of caller-owned tensors can corrupt intermediate activations or training data if the same tensor is used elsewhere after calling this utility.

11. core/models/modules/ops/fused_act.py (line 23): In the CPU branch of fused_leaky_relu, F.leaky_relu(..., negative_slope=0.2) uses a hardcoded 0.2 instead of the negative_slope parameter passed to the function. The parameter is respected only in the CUDA path via FusedLeakyReLUFunction.

Any caller that sets a non-default negative_slope will get silently wrong activations when running on CPU, causing training/inference results to diverge between CPU and GPU.

12. core/models/utils.py (line 8): In NoiseManager.__init__, the None-check if not None in noise is placed inside the for loop but is loop-invariant; it is evaluated once per iteration despite never changing. More critically, multiple noise tensors with the same last spatial dimension will collide in noise_lut (later entries silently overwrite earlier ones).

The invariant check wastes iterations, and the size-keyed lookup table can silently drop noise tensors for layers that share a spatial resolution, injecting the wrong noise into those layers.

13. core/loss/non_saturating_gan_loss.py (line 37): In reg_d, real.requires_grad = True modifies the requires_grad attribute of the tensor passed in from outside. If the caller passes a tensor from the teacher network's forward pass, this side-effect enables gradient tracking on it unexpectedly in subsequent uses.

Setting requires_grad directly on an externally-owned tensor is a side effect that persists after the call, potentially causing unintended gradient flow into the teacher network or memory leaks.

14. core/models/modules/mobile_synthesis_block.py (line 37): Both self.up and self.conv1 receive the same style slice style[:, 0, :]. The block's wsize() returns 3 (slots 0, 1, 2), yet up and conv1 share slot 0, leaving slot 1 used only by conv2 and slot 2 by to_img. If the intended design was for conv1 to use its own independent style vector, this is a modulation bug.

Sharing a style vector between the IDWT upsample and the subsequent convolution reduces the effective style dimensionality of the block and may degrade image quality.

15. demo.py (line 14): distiller is used for inference in the demo loop without calling distiller.eval() or wrapping in torch.no_grad(). While mapping_net and synthesis_net are set to eval individually, the student module remains in training mode with active dropout/batchnorm behavior and unnecessary gradient computation.

Running the student network in training mode causes BatchNorm to use batch statistics rather than running statistics, producing inconsistent outputs, and wastes memory computing gradients that are never used.

---

Review generated automatically.