Skip to content

fix(stg): drop unsupported momentum_buffer kwarg from APG call (#492) - #555

Open
Tai An (Anai-Guo) wants to merge 1 commit into
Lightricks:masterfrom
Anai-Guo:fix/apg-momentum-buffer-kwarg
Open

fix(stg): drop unsupported momentum_buffer kwarg from APG call (#492)#555
Tai An (Anai-Guo) wants to merge 1 commit into
Lightricks:masterfrom
Anai-Guo:fix/apg-momentum-buffer-kwarg

Conversation

@Anai-Guo

Copy link
Copy Markdown

Fixes #492

Problem

STGGuiderAdvanced.predict_noise calls apg() with a momentum_buffer=None
keyword that apg() does not accept:

# stg.py:55
def apg(
    noise_pred_pos: torch.Tensor,
    noise_pred_neg: torch.Tensor,
    cfg_scale: float,
    eta: float = 1.0,
    norm_threshold: float = 0.0,
):
# stg.py:459 -- reached on every sampling step when apply_apg=True
if self.apply_apg:
    stg_result = apg(
        stg_result,
        noise_pred_neg,
        cfg_scale=self.apg_cfg_scale,
        momentum_buffer=None,   # <-- not a parameter of apg()
        eta=self.eta,
        norm_threshold=self.norm_threshold,
    )

apply_apg is a node input on LTXVApplySTGAdvanced (stg.py:626), so turning
it on makes sampling fail immediately with

TypeError: apg() got an unexpected keyword argument 'momentum_buffer'

which is exactly what #492 reports.

Why removing it is the right fix

  • apg()'s body never references momentum at all, so the argument was inert
    even before it started raising — passing None could not have had an effect.
  • The sibling call site, STGGuiderAPG.predict_noise at stg.py:857, already
    calls apg(noise_pred_pos, noise_pred_neg, self.cfg_scale, self.eta, self.norm_threshold) with no momentum argument. Of the two apg() call sites
    in the file, only this one disagrees with the signature.
  • It is also the workaround the reporter of apg() got unexpected keyword argument 'momentum_buffer' when apply_apg=True #492 confirmed works.

Restoring real momentum support would be a behaviour change (MomentumBuffer is
constructed in STGGuiderAPG.__init__ and reset on timestep increase, but is
never threaded into apg()), so I left that alone — happy to open a separate
issue for it if you'd like it tracked.

Verification

No ComfyUI runtime needed: project() and apg() were lifted out of stg.py
with ast and executed as-is against real tensors.

apg signature: (noise_pred_pos, noise_pred_neg, cfg_scale, eta=1.0, norm_threshold=0.0)

[1] STGGuiderAdvanced.predict_noise, stg.py:460 -- as shipped
    TypeError: apg() got an unexpected keyword argument 'momentum_buffer'
    matches issue #492 verbatim: True

[2] stg.py:460 with `momentum_buffer=None,` removed (this PR)
    returns (1, 4, 8, 8) torch.float32

[3] STGGuiderAPG.predict_noise, stg.py:857 -- sibling, unmodified
    returns (1, 4, 8, 8)
    fixed call == sibling call: True

[4] does apg's body reference momentum at all?
    'momentum' appears in apg() body: False

So the unpatched path reproduces the reported error verbatim, and the patched
call is bit-identical to the already-working STGGuiderAPG path.

Notes

🤖 Generated with Claude Code

STGGuiderAdvanced.predict_noise passes momentum_buffer=None to apg(),
but apg() is defined as

    def apg(noise_pred_pos, noise_pred_neg, cfg_scale, eta=1.0, norm_threshold=0.0)

so every sampling step with apply_apg=True dies with

    TypeError: apg() got an unexpected keyword argument 'momentum_buffer'

The keyword was inert regardless: apg()'s body never references momentum.
The sibling call in STGGuiderAPG.predict_noise already omits it, and
dropping it here makes the two call sites agree.

Fixes Lightricks#492
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.

apg() got unexpected keyword argument 'momentum_buffer' when apply_apg=True

1 participant