Gpl: change bin size from int to float #6530
Conversation
…double avoid rounding errors Signed-off-by: Lucas Yuki Imamura <[email protected]>
Signed-off-by: Lucas Yuki Imamura <[email protected]>
Signed-off-by: Lucas Yuki Imamura <[email protected]>
Signed-off-by: LucasYuki <[email protected]>
Signed-off-by: LucasYuki <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
|
@LucasYuki Is this ready for review? Please fix clang-format too. |
Signed-off-by: LucasYuki <[email protected]>
Signed-off-by: LucasYuki <[email protected]>
src/gpl/src/nesterovBase.cpp
Outdated
| @@ -895,7 +893,7 @@ void BinGrid::updateBinsGCellDensityArea(const std::vector<GCellHandle>& cells) | |||
| bin.setDensity((static_cast<float>(bin.instPlacedArea()) | |||
| + static_cast<float>(bin.fillerArea()) | |||
| + static_cast<float>(bin.nonPlaceArea())) | |||
| / scaledBinArea); | |||
| / binArea); | |||
There was a problem hiding this comment.
@maliberty I talked to you about this change. Other code changes are to fix the negative bin sizes.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
src/gpl/src/graphics.cpp
Outdated
| x_grid_set.push_back(bin[x].ux()); | ||
| } | ||
| for (int y = 0; y < y_grid; y++) { | ||
| y_grid_set.push_back(bin[y*x_grid].uy()); |
There was a problem hiding this comment.
warning: performing an implicit widening conversion to type 'std::vector::size_type' (aka 'unsigned long') of a multiplication performed in type 'int' [bugprone-implicit-widening-of-multiplication-result]
y_grid_set.push_back(bin[y*x_grid].uy());
^Additional context
src/gpl/src/graphics.cpp:493: make conversion explicit to silence this warning
y_grid_set.push_back(bin[y*x_grid].uy());
^src/gpl/src/graphics.cpp:493: perform multiplication in a wider type
y_grid_set.push_back(bin[y*x_grid].uy());
^this looks like a mistake Signed-off-by: Augusto Berndt <[email protected]>
Signed-off-by: Augusto Berndt <[email protected]>
| x_grid_set.push_back(bin[x].ux()); | ||
| } | ||
| for (int y = 0; y < y_grid; y++) { | ||
| y_grid_set.push_back(bin[y * x_grid].uy()); |
There was a problem hiding this comment.
warning: performing an implicit widening conversion to type 'std::vector::size_type' (aka 'unsigned long') of a multiplication performed in type 'int' [bugprone-implicit-widening-of-multiplication-result]
y_grid_set.push_back(bin[y * x_grid].uy());
^Additional context
src/gpl/src/graphics.cpp:495: make conversion explicit to silence this warning
y_grid_set.push_back(bin[y * x_grid].uy());
^src/gpl/src/graphics.cpp:495: perform multiplication in a wider type
y_grid_set.push_back(bin[y * x_grid].uy());
^…aledBinArea Signed-off-by: Augusto Berndt <[email protected]>
Signed-off-by: Augusto Berndt <[email protected]>
Signed-off-by: LucasYuki <[email protected]>
Signed-off-by: LucasYuki <[email protected]>
| x_grid_set.push_back(bin[x].ux()); | ||
| } | ||
| for (int y = 0; y < y_grid; y++) { | ||
| y_grid_set.push_back(bin[y * x_grid].uy()); |
There was a problem hiding this comment.
warning: performing an implicit widening conversion to type 'std::vector::size_type' (aka 'unsigned long') of a multiplication performed in type 'int' [bugprone-implicit-widening-of-multiplication-result]
y_grid_set.push_back(bin[y * x_grid].uy());
^Additional context
src/gpl/src/graphics.cpp:493: make conversion explicit to silence this warning
y_grid_set.push_back(bin[y * x_grid].uy());
^src/gpl/src/graphics.cpp:493: perform multiplication in a wider type
y_grid_set.push_back(bin[y * x_grid].uy());
^
Signed-off-by: LucasYuki <[email protected]>
Signed-off-by: LucasYuki <[email protected]>
Signed-off-by: LucasYuki <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
| x_grid_set.push_back(bin[0].lx()); | ||
| y_grid_set.push_back(bin[0].ly()); | ||
|
|
||
| for (std::vector<Bin>::size_type x = 0; x < x_grid; x++) { |
There was a problem hiding this comment.
Why are you using std::vector<Bin>::size_type when your x_grid limit for the loop is an int?
There was a problem hiding this comment.
It was to implement this clang-tidy suggestion, but in the x_grid it was not necessary.
Do I change back to int?
| x_grid_set.push_back(bin[0].lx()); | ||
| y_grid_set.push_back(bin[0].ly()); | ||
|
|
||
| for (std::vector<Bin>::size_type x = 0; x < x_grid; x++) { |
There was a problem hiding this comment.
Why are you using std::vector<Bin>::size_type when your x_grid limit for the loop is an int?
|
Is there an ORFS PR this needs to pair with? |
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: LucasYuki <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
Signed-off-by: Augusto Berndt <[email protected]>
| bins_[bin_index] | ||
| = Bin(idxX, idxY, bin_lx, bin_ly, bin_ux, bin_uy, targetDensity_); | ||
| auto& bin = bins_[bin_index]; | ||
| if (bin.dx() < 0 || bin.dy() < 0) |
There was a problem hiding this comment.
warning: statement should be inside braces [google-readability-braces-around-statements]
| if (bin.dx() < 0 || bin.dy() < 0) | |
| if (bin.dx() < 0 || bin.dy() < 0) { |
src/gpl/src/nesterovBase.cpp:792:
- bin.dy());
+ bin.dy());
+ }
The bin size was calculated by dividing the size of the CoreBBox by the number of bins and then rounding it up.
This can cause some bins to have a negative size.
Changes: