Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

utl:: check if container is empty on utl::shuffle #6446

Merged
merged 3 commits into from
Dec 30, 2024

Conversation

eder-matheus
Copy link
Collaborator

Fix Coverage errors

Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
Copy link
Contributor

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

Comment on lines 2498 to 2500
if (!congestionGridsH.empty()) {
utl::shuffle(congestionGridsH.begin(), congestionGridsH.end(), g);
}
Copy link
Member

Choose a reason for hiding this comment

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

It would be better to make shuffle handle the empty vector correctly so that all users don't need this check.

Copy link
Member

Choose a reason for hiding this comment

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

if (n == 0) return; should do it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks! I'm testing it right now and will add it to the PR soon.

Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
Signed-off-by: Eder Monteiro <emrmonteiro@precisioninno.com>
@eder-matheus eder-matheus changed the title grt: check if congestion grid vectors are empty before shuffling them utl:: check if container is empty on utl::shuffle Dec 30, 2024
Copy link
Contributor

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

Comment on lines -2498 to +2499
utl::shuffle(congestionGridsV.begin(), congestionGridsV.end(), g);
utl::shuffle(congestionGridsH.begin(), congestionGridsH.end(), g);
utl::shuffle(congestionGridsV.begin(), congestionGridsV.end(), g);
Copy link
Member

Choose a reason for hiding this comment

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

Is this just a leftover or is there some significance to the order of these lines?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just a leftover.

@eder-matheus eder-matheus merged commit 13adf69 into The-OpenROAD-Project:master Dec 30, 2024
11 checks passed
@eder-matheus eder-matheus deleted the grt_check_cong branch December 30, 2024 17:54
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.

2 participants