Skip to content

mpl: always use best valid result#6967

Merged
eder-matheus merged 7 commits into
The-OpenROAD-Project:masterfrom
AcKoucher:mpl-use-best-result
May 27, 2025
Merged

mpl: always use best valid result#6967
eder-matheus merged 7 commits into
The-OpenROAD-Project:masterfrom
AcKoucher:mpl-use-best-result

Conversation

@AcKoucher

@AcKoucher AcKoucher commented Apr 1, 2025

Copy link
Copy Markdown
Contributor

Context

When running Secure-CI for #6649, some results were considerably different than master which doesn't seem to make sense as there's no functionality changes.

Cause of the Differences

Inside SA, we have a mechanism to store the best valid result. It was created in order to ensure MPL converges if a valid result was generated at any point during annealing. However, this best stored result is only actually used if the final result is invalid. We should use the best valid result even if the final iteration result is a valid one, because we might have generated a good result during the early iterations that ended up being discarded.

I.e., what's happening in the previously cited PR is that the change in the random number generator affected the path SA takes and some good results from early iterations that we were luckily choosing are now being discarded.

Changes

Use the best valid result stored even if the final result is valid.
With these changes, the results should improve compared to master and the results between base/test for #6649 should get more consistent.

Additional

The io_constraints3 test is not using the boundary penalty in SA. That will make the test more concise as the centralization attempt should always work regardless of the shape chosen for the std cell cluster.

Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
@github-actions

github-actions Bot commented Apr 1, 2025

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@QuantamHD

Copy link
Copy Markdown
Collaborator

Noice!

@maliberty

Copy link
Copy Markdown
Member

Are you running a secure CI for this? Hopefully we just see improvements...

@AcKoucher

AcKoucher commented Apr 2, 2025

Copy link
Copy Markdown
Contributor Author

I ran a Secure-CI and it had some failures. I'm taking a look.

Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@AcKoucher

Copy link
Copy Markdown
Contributor Author

Re-running Secure-CI.

@AcKoucher

Copy link
Copy Markdown
Contributor Author

uW is failing due to high congestion in GRT post-drt. I'm taking a look.

@AcKoucher

AcKoucher commented May 2, 2025

Copy link
Copy Markdown
Contributor Author

Re-ran CI with a lower routing layer adjustment for uW (0.15 -> 0.10) and it passed. However, ngt45/swerv_wrapper is failing in detail placement. I'll investigate.

AcKoucher added 3 commits May 5, 2025 14:05
Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
   1) Change test to not consider boundary penalty in
      order to make it more concise.
   2) Adapt regression tests results.

Signed-off-by: Arthur Koucher <arthurkoucher@precisioninno.com>
@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@AcKoucher

AcKoucher commented May 23, 2025

Copy link
Copy Markdown
Contributor Author

The IO constraints project was being prioritized so this was on hold.

I updated the branch with master and adapted the tests results. I'm also updating a test's configuration to make it more concise.

Re-running Secure-CI.

@AcKoucher

Copy link
Copy Markdown
Contributor Author

ngt45/swerv_wrapper is faling during GRT due high congestion. The only difference with master is that the order of the small RAMs changed inside their array. I'm taking a look.

@AcKoucher

Copy link
Copy Markdown
Contributor Author

#3190 for decreasing routing layer adjustment on ngt45/swerv_wrapper. Running final Secure-CI to double-check everything.

@AcKoucher AcKoucher requested a review from eder-matheus May 26, 2025 19:01
@AcKoucher

Copy link
Copy Markdown
Contributor Author

@eder-matheus Secure-CI passed.

@eder-matheus eder-matheus merged commit dbe7a4e into The-OpenROAD-Project:master May 27, 2025
11 checks passed
@AcKoucher AcKoucher deleted the mpl-use-best-result branch May 27, 2025 15:22
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.

4 participants