Skip to content

[URGENT] Bring back commented param sw for conv structure and perf#2344

Merged
dorde-antic merged 1 commit intodevelopfrom
paramsw-urgent
Apr 15, 2026
Merged

[URGENT] Bring back commented param sw for conv structure and perf#2344
dorde-antic merged 1 commit intodevelopfrom
paramsw-urgent

Conversation

@dorde-antic
Copy link
Copy Markdown
Contributor

@dorde-antic dorde-antic commented Apr 15, 2026

Motivation

It was commented out for testing purposes of Attention Parameter Sweeps and wasn't uncommented.

Technical Details

Uncommented

                                                        parameterSweep("conv_structure")
                                                        parameterSweep("perf_config")

Test Plan

No need to wait for CI checks, lets merge this asap so that all param sw steps are kicked on weekly.

Test Result

@dorde-antic dorde-antic requested a review from causten as a code owner April 15, 2026 10:49
@dorde-antic dorde-antic added the skip-ci Don't build Jenkins tests label Apr 15, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Code Review

The change is straightforward and correct — restoring two previously-commented-out parameterSweep calls that were accidentally left disabled after Attention Parameter Sweep testing.

What the change does:

  • Uncomments parameterSweep("conv_structure") and parameterSweep("perf_config") inside the "Parameter Sweep" stage of the weekly Jenkins matrix.
  • These calls use the default sweep type, which runs python3 ./bin/parameterSweeps.py -j ${limit_lit_workers} ${CONFIG} --log-failures.

One thing worth noting:
The "Parameter Sweep" stage lives inside a matrix that iterates over all six codepaths (mfma, vanilla, navi21, navi3x, navi4x, gfx950). Since parameterSweep("conv_structure") and parameterSweep("perf_config") do not use the CODEPATH variable, they will execute the same sweep script six times (once per matrix row). This appears to be the pre-existing behavior that was in place before the lines were commented out, so this PR restores it faithfully — but it is worth confirming that running these config-agnostic sweeps once per codepath is intentional and not an accidental duplication from when the matrix was introduced.

No functional issues with the change itself. The skip-ci label is appropriate since this only modifies the CI pipeline definition.

Open in Web View Automation 

Sent by Cursor Automation: Djordje’s Automation

//parameterSweep("conv_structure")
//parameterSweep("perf_config")
parameterSweep("conv_structure")
parameterSweep("perf_config")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this stage runs inside the CODEPATH matrix (over mfma, vanilla, navi21, navi3x, navi4x, gfx950), both parameterSweep("conv_structure") and parameterSweep("perf_config") will execute six times — once per matrix row — running the same command each time because neither call uses the CODEPATH variable.

If the intent is to run these sweeps only once (codepath-independent), they could be moved to a separate non-matrix stage. If the current behavior (6× execution) is intentional and was how it worked before being commented out, this is fine to merge as-is.

Copy link
Copy Markdown
Member

@umangyadav umangyadav left a comment

Choose a reason for hiding this comment

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

Make sure weekly passes for merging this

@dorde-antic
Copy link
Copy Markdown
Contributor Author

@umangyadav we already had this on develop (until few days ago) and weekly passed with it,
now weekly fails because of failing attn ones, which is what we actually want so that we don't get fake green weekly while having attn bugs unresolved

@dorde-antic dorde-antic removed the skip-ci Don't build Jenkins tests label Apr 15, 2026
@dorde-antic dorde-antic merged commit 5e1d9a7 into develop Apr 15, 2026
14 checks passed
@dorde-antic dorde-antic deleted the paramsw-urgent branch April 15, 2026 14:31
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.

3 participants