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

✨ Parameter to move inputs/outputs to top/bottom border after hexagonalization #692

Open
wants to merge 52 commits into
base: main
Choose a base branch
from

Conversation

simon1hofmann
Copy link
Collaborator

@simon1hofmann simon1hofmann commented Mar 13, 2025

Description

Parameters to move inputs to top border or outputs to the bottom border after hexagonalization

Checklist:

  • The pull request only contains commits that are related to it.
  • I have added appropriate tests and documentation.
  • I have added a changelog entry.
  • I have created/adjusted the Python bindings for any new or updated functionality.
  • I have made sure that all CI jobs on GitHub pass.
  • The pull request introduces no new warnings and follows the project's style guidelines.

Sorry, something went wrong.

@simon1hofmann simon1hofmann self-assigned this Mar 13, 2025
@simon1hofmann simon1hofmann added the enhancement New feature or request label Mar 13, 2025
Copy link

codecov bot commented Mar 13, 2025

Codecov Report

Attention: Patch coverage is 98.48101% with 6 lines in your changes missing coverage. Please review.

Project coverage is 98.19%. Comparing base (0ee0f3d) to head (324eaa2).

Files with missing lines Patch % Lines
...on/algorithms/physical_design/hexagonalization.hpp 97.89% 6 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #692      +/-   ##
==========================================
- Coverage   98.20%   98.19%   -0.01%     
==========================================
  Files         255      255              
  Lines       42229    42519     +290     
  Branches     1949     1989      +40     
==========================================
+ Hits        41471    41753     +282     
- Misses        758      766       +8     
Files with missing lines Coverage Δ
include/fiction/algorithms/path_finding/a_star.hpp 100.00% <100.00%> (ø)
...on/algorithms/physical_design/wiring_reduction.hpp 98.76% <100.00%> (ø)
include/fiction/utils/placement_utils.hpp 85.71% <100.00%> (+0.71%) ⬆️
...st/algorithms/physical_design/hexagonalization.cpp 100.00% <100.00%> (ø)
test/utils/blueprints/layout_blueprints.hpp 100.00% <100.00%> (ø)
...on/algorithms/physical_design/hexagonalization.hpp 97.61% <97.89%> (-2.39%) ⬇️

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ee0f3d...324eaa2. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

simon1hofmann and others added 13 commits March 13, 2025 16:13

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Signed-off-by: GitHub Actions <[email protected]>
Signed-off-by: GitHub Actions <[email protected]>
@simon1hofmann simon1hofmann requested a review from marcelwa March 14, 2025 11:18
@marcelwa marcelwa requested a review from Copilot March 14, 2025 12:18
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request introduces a parameter to move inputs to the top border after hexagonalization and adds new tests to verify the behavior with parameters and statistics.

  • Adds new tests under hexagonalization to check behavior when placing inputs in the top row
  • Exports new functions (hexagonalization_params, hexagonalization_stats, and hexagonalization_route_inputs_error) in the module's init.py

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

File Description
bindings/mnt/pyfiction/test/algorithms/physical_design/test_hexagonalization.py New tests to verify hexagonalization with parameters and statistics
bindings/mnt/pyfiction/init.py Updated exports to include new hexagonalization functionality

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…xagonalization.py

Co-authored-by: Copilot <[email protected]>
Signed-off-by: simon1hofmann <[email protected]>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Co-authored-by: Marcel Walter <[email protected]>
Signed-off-by: simon1hofmann <[email protected]>
simon1hofmann and others added 4 commits March 19, 2025 18:25
Signed-off-by: GitHub Actions <[email protected]>
@simon1hofmann
Copy link
Collaborator Author

@marcelwa @samuelshng
Extending output wires to the bottom border is now possible using the -o flag (-o 0 for no extension, -o 1 for extending to the bottom row, and -o 2 for extending to the bottom row with planar rerouting).

For example to hexagonalize a layout and route inputs to the top row and outputs to the bottom row (allowing crossings) you can simply use: hex -i 1 -o 1.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@marcelwa
Copy link
Collaborator

@marcelwa @samuelshng
Extending output wires to the bottom border is now possible using the -o flag (-o 0 for no extension, -o 1 for extending to the bottom row, and -o 2 for extending to the bottom row with planar rerouting).

For example to hexagonalize a layout and route inputs to the top row and outputs to the bottom row (allowing crossings) you can simply use: hex -i 1 -o 1.

Many thanks for implementing this so quickly 🙏 from a user's perspective, I would have expected this functionality to be available via flags instead of parameters. In the current setup, a user might enter hex -i 3 -o 5, which is obviously nonsensical. Since I cannot think of a situation in which someone would want only PI wires to be crossing-free but not PO wires (or vice versa), I expected a flag-based interface like hex -i -o -p, i.e., extend PIs and POs, and do so planarly.

What do you think? Is there an argument to be had against it?

@simon1hofmann
Copy link
Collaborator Author

@marcelwa @samuelshng
Extending output wires to the bottom border is now possible using the -o flag (-o 0 for no extension, -o 1 for extending to the bottom row, and -o 2 for extending to the bottom row with planar rerouting).
For example to hexagonalize a layout and route inputs to the top row and outputs to the bottom row (allowing crossings) you can simply use: hex -i 1 -o 1.

Many thanks for implementing this so quickly 🙏 from a user's perspective, I would have expected this functionality to be available via flags instead of parameters. In the current setup, a user might enter hex -i 3 -o 5, which is obviously nonsensical. Since I cannot think of a situation in which someone would want only PI wires to be crossing-free but not PO wires (or vice versa), I expected a flag-based interface like hex -i -o -p, i.e., extend PIs and POs, and do so planarly.

What do you think? Is there an argument to be had against it?

Yes makes sense, I will update the cli accordingly. I will still leave the implementation in the C++/Python code as is to still offer the possibility of only having planar routings for either PIs or POs.

simon1hofmann and others added 3 commits March 20, 2025 13:36
…xagonalization' into route_pis_to_top_border_after_hexagonalization

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was signed with the committer’s verified signature.
marcelwa Marcel Walter
@marcelwa
Copy link
Collaborator

@simon1hofmann many thanks for the great addition! 🙏 I've got some minor feedback and implemented a few small inconsistency fixes myself. Additionally, there seems to be inconsistent behavior with the -p flag. Could you have a look?

simon1hofmann and others added 9 commits March 23, 2025 14:04

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Co-authored-by: Marcel Walter <[email protected]>
Signed-off-by: simon1hofmann <[email protected]>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Signed-off-by: GitHub Actions <[email protected]>
@simon1hofmann simon1hofmann requested a review from marcelwa March 23, 2025 20:24
simon1hofmann and others added 3 commits March 25, 2025 10:20
Signed-off-by: GitHub Actions <[email protected]>

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@simon1hofmann simon1hofmann requested a review from marcelwa March 25, 2025 11:28
@simon1hofmann simon1hofmann changed the title ✨ Parameter to move inputs to top border after hexagonalization ✨ Parameter to move inputs/outputs to top/bottom border after hexagonalization Mar 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants