Skip to content

fix: fix bug that transolver uses Transolver attention, not regular d…#166

Merged
MauritsBleeker merged 3 commits into
mainfrom
bug/use-transolver-attention-for-transolver-model
Apr 16, 2026
Merged

fix: fix bug that transolver uses Transolver attention, not regular d…#166
MauritsBleeker merged 3 commits into
mainfrom
bug/use-transolver-attention-for-transolver-model

Conversation

@MauritsBleeker
Copy link
Copy Markdown
Collaborator

A bug slipped in: the Transolver architecture was never using the Transolver attention as intended, but instead using regular dot-product attention. I fix this by adding the transolver attention constructor string to the Transolver schema after validation is done. Next, I added a deterministic regression test for Transolver that also checks whether we use the correct attention class.

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@MauritsBleeker MauritsBleeker changed the title bug: fix bug that transolver uses Transolver attention, not regular d… fix: fix bug that transolver uses Transolver attention, not regular d… Apr 16, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 16, 2026

Coverage

Tests Skipped Failures Errors Time
1259 24 💤 0 ❌ 0 🔥 24.478s ⏱️

Copy link
Copy Markdown
Collaborator

@HennerM HennerM left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +19 to +20
def set_condition_dim(self) -> "TransolverConfig":
"""Set condition_dim in transformer_block_config based on data_specs."""
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.

Suggested change
def set_condition_dim(self) -> "TransolverConfig":
"""Set condition_dim in transformer_block_config based on data_specs."""
def set_attention_constructor(self) -> "TransolverConfig":
"""Set attention_constructor in transformer_block_config."""

Comment on lines +30 to +31
def set_condition_dim(self) -> "TransolverPlusPlusConfig":
"""Set condition_dim in transformer_block_config based on data_specs."""
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.

Suggested change
def set_condition_dim(self) -> "TransolverPlusPlusConfig":
"""Set condition_dim in transformer_block_config based on data_specs."""
def set_attention_constructor(self) -> "TransolverPlusPlusConfig":
"""Set attention_constructor in transformer_block_configto transolver++."""

assert actual_sum == pytest.approx(expected_sum, abs=1e-5)


def test_transolver_determinism_regression_check(
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.

nice!

@MauritsBleeker MauritsBleeker merged commit 6d79261 into main Apr 16, 2026
9 checks passed
@MauritsBleeker MauritsBleeker deleted the bug/use-transolver-attention-for-transolver-model branch April 16, 2026 11:41
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants