Fix DelayBuffer state mutation on invalid lag

[Xalzeroph]

Description

Fixes #7793

DelayBuffer.set_time_lag currently writes the requested lag into the live tensor before validating the resulting range. If validation raises, the invalid value remains in the buffer and later compute() calls use it.

This stages the update in a cloned tensor, validates the complete candidate state, and commits it with copy_() only after validation succeeds. The existing tensor identity is preserved for callers that retain a reference to time_lags.

Type of change

• Bug fix (non-breaking change which fixes an existing issue)

Checklist

• I have run the pre-commit checks
• I have added regression coverage for the rejected scalar and tensor updates
• I have added a changelog fragment

Local checks: python -m compileall and git diff --check passed. I could not run the unit test here because PyTorch is not installed and the uv-managed environment could not bootstrap in this container.

[isaaclab-review-bot]

Isaac Lab Review Bot

The change makes DelayBuffer.set_time_lag transactional: updates are staged, validated as a complete candidate state, and committed only after validation succeeds. Rejected scalar and tensor lags therefore leave the live lag tensor and cached extrema unchanged.

• Design and architecture: State remains owned by DelayBuffer, with the committed lag tensor and cached minimum/maximum updated together. Using copy_() preserves the existing tensor identity, while compute() continues to consume the same _time_lags object.
• API: Accepted inputs, validation errors, device conversion, and successful update semantics remain unchanged. The observable behavior on failed validation is corrected without renaming or removing public APIs, and the package includes an appropriate past-tense changelog fragment.
• Implementation: Both scalar and tensor updates now target a clone before range validation, and the live state is committed only on success. Regression coverage checks rejected negative and over-limit updates. The added clone introduces a small per-update allocation, and the new test still warrants execution in the project environment because it was not run by the author, but neither issue requires a pre-merge correction based on the supplied patch.

No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.

Automated review; human maintainers own approval decisions.

[greptile-apps]

Confidence Score: 4/5

The implementation appears safe to merge; the only concern is a non-blocking gap in regression coverage for retained time_lags tensor identity.

Findings

1.  Tensor identity remains untested ▶

Summary

This PR makes DelayBuffer.set_time_lag transactional by staging lag updates in a cloned tensor, validating the complete candidate, and committing it in place only when valid.

• Prevents rejected scalar and tensor updates from mutating live buffer state or cached extrema.
• Preserves the identity of the publicly exposed time_lags tensor through an in-place commit.
• Adds regression coverage for negative, oversized, and mixed-validity partial updates, though the identity guarantee itself remains unasserted.
• Adds a matching bug-fix changelog fragment.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Requested lag update] --> B[Clone current time_lags]
  B --> C[Apply update to candidate]
  C --> D[Compute and validate candidate extrema]
  D -->|Invalid| E[Raise error<br/>live state unchanged]
  D -->|Valid| F[copy_ candidate into live tensor]
  F --> G[Commit cached min and max]

Loading

_{Reviews (1) · Last reviewed commit: "Add changelog for invalid lag state fix"}

[greptile-apps]

Tensor identity remains untested

This regression test verifies only the tensor values and cached extrema, while the implementation explicitly preserves time_lags identity for callers that retain a reference. Retain the pre-call tensor and assert that it remains the same object after the rejected update. Otherwise, a future change from copy_() to tensor reassignment could break that compatibility guarantee while this test continues to pass.

Suggested change

	expected_lags = delay_buffer.time_lags.clone()
	with pytest.raises(ValueError):
	delay_buffer.set_time_lag(time_lag, batch_ids)
	assert torch.equal(delay_buffer.time_lags, expected_lags)
	expected_lags = delay_buffer.time_lags
	expected_values = expected_lags.clone()
	with pytest.raises(ValueError):
	delay_buffer.set_time_lag(time_lag, batch_ids)
	assert delay_buffer.time_lags is expected_lags
	assert torch.equal(delay_buffer.time_lags, expected_values)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!