Skip to content

Fix latent bugs in tomography optimizer wiring, constraints, and radon#239

Merged
arthurmccray merged 1 commit into
devfrom
tomography-bug-fixes
Jun 1, 2026
Merged

Fix latent bugs in tomography optimizer wiring, constraints, and radon#239
arthurmccray merged 1 commit into
devfrom
tomography-bug-fixes

Conversation

@cedriclim1

Copy link
Copy Markdown
Collaborator

What problem this PR addreseses

step_optimizers and zero_grad_all looped over optimizer_params and stepped both the object and pose optimizers on every pass, so with pose optimization enabled each optimizer took two Adam steps per batch. Gate by key, matching step_schedulers.

Also:

  • pass num_iter to set_schedulers on the reset_dset path so cosine/linear/ exponential schedulers get a valid T_max
  • scheduler_params setter no longer mutates the caller's dict
  • drop the non-existent tv_plane key from ObjINRConstraints.soft_constraint_keys (it crashed Constraints.str)
  • ObjectPixelated.get_tv_loss now takes TV over the trailing spatial dims, so it handles a 3D volume, obj_view's [1, D, H, W], and a multimodal [C, D, H, W]
  • remove an unreachable duplicate branch in TomographyINRDataset.forward
  • align iradon_torch's default theta with radon_torch / scikit-image (endpoint excluded)

Add regression tests covering each fix.

What Reviewer Should Do

Nothing - all tests pass checking for these fixes.

step_optimizers and zero_grad_all looped over optimizer_params and stepped
both the object and pose optimizers on every pass, so with pose optimization
enabled each optimizer took two Adam steps per batch. Gate by key, matching
step_schedulers.

Also:
- pass num_iter to set_schedulers on the reset_dset path so cosine/linear/
  exponential schedulers get a valid T_max
- scheduler_params setter no longer mutates the caller's dict
- drop the non-existent tv_plane key from ObjINRConstraints.soft_constraint_keys
  (it crashed Constraints.__str__)
- ObjectPixelated.get_tv_loss now takes TV over the trailing spatial dims, so it
  handles a 3D volume, obj_view's [1, D, H, W], and a multimodal [C, D, H, W]
- remove an unreachable duplicate branch in TomographyINRDataset.forward
- align iradon_torch's default theta with radon_torch / scikit-image (endpoint
  excluded)

Add regression tests covering each fix.

@arthurmccray arthurmccray left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@arthurmccray arthurmccray merged commit f51cace into dev Jun 1, 2026
4 checks passed
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