Skip to content

[AP] Removed Old Cluster-Level AP Flow #3054

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

Conversation

AlexandreSinger
Copy link
Contributor

Prior to the flat AP flow, a cluster-level AP flow existed in VPR which performed a SimPL-style algorithm on the clusters created during packing before performing a placement quench.

Although well-written, this flow was not shown to outperform the SA placer in VPR. It has also been becoming confusing to keep in VPR since the new flat AP flow supercedes it. It is unclear if a cluster-level AP flow will work well with the flat AP flow; however in that case the cluster-level AP flow would be made using the new AP APIs written.

Removed the old cluster-level AP flow to reduce confusion.

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code build Build system lang-make CMake/Make code labels May 17, 2025
@AlexandreSinger
Copy link
Contributor Author

@vaughnbetz What is your opinion on this? I recall we discussed this a month or two ago. I think it is tricky to keep this code around in Master while the flat AP flow is becoming stronger. We can always find this code again by going to the previous releases of VTR (however, this AP flow was never added to the documentation as far as I could tell; it only appeared in the code).

Prior to the flat AP flow, a cluster-level AP flow existed in VPR which
performed a SimPL-style algorithm on the clusters created during packing
before performing a placement quench.

Although well-written, this flow was not shown to outperform the SA
placer in VPR. It has also been becoming confusing to keep in VPR since
the new flat AP flow supercedes it. It is unclear if a cluster-level AP
flow will work well with the flat AP flow; however in that case the
cluster-level AP flow would be made using the new AP APIs written.

Removed the old cluster-level AP flow to reduce confusion.
@AlexandreSinger AlexandreSinger force-pushed the feature-remove-old-ap-flow branch from 6448a40 to 175da00 Compare May 18, 2025 02:23
@vaughnbetz
Copy link
Contributor

Looks good. One question: you're removing the cmakelists check for analytic placement, which brings in eigen. Do you have a separate such check for your new analytic placer or do you just always pull in eigen?

@vaughnbetz vaughnbetz merged commit 87eefdb into verilog-to-routing:master May 19, 2025
36 checks passed
@AlexandreSinger
Copy link
Contributor Author

Looks good. One question: you're removing the cmakelists check for analytic placement, which brings in eigen. Do you have a separate such check for your new analytic placer or do you just always pull in eigen?

The CMake check that I removed does not bring in Eigen. It only checks if Eigen is installed and turns on a compiler directive for the cluster-level Analytical Placement flow (which I purposefully did not use). In the CMakeLists for VPR there is another compiler directive called "EIGEN_INSTALLED" which will change the compile for certain portions of the atom-level AP flow (i.e. the solver).

The CI also tests both Eigen not being installed and Eigen being installed.

@AlexandreSinger AlexandreSinger deleted the feature-remove-old-ap-flow branch May 19, 2025 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Build system lang-cpp C/C++ code lang-make CMake/Make code VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants