Skip to content

[packer] Changing List of Feasible Candidates to Priority Queue #2994

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

RonZ13
Copy link

@RonZ13 RonZ13 commented Apr 22, 2025

Description

[packer] Changing the vector of candidate molecules into LazyPopUniquePriorityQueue.

The class LazyPopUniquePriorityQueue is a priority queue that allows for lazy deletion of elements.
The elements are pair of key and sort-value. The key is a unique value to identify the item, and the sort-value is used to sort the item.
It is implemented using a vector and 2 sets, one set keeps track of the elements in the queue, and the other set keeps track of the elements that are pending deletion, so that they can be removed from the queue when they are popped.
The class definiation can be found in vpr/src/util/lazy_pop_unique_priority_queue.h

Currently, the class supports the following functions:
    LazyPopUniquePriorityQueue::push(): Pushes a key-sort-value (K-SV) pair into the priority queue and adds the key to the tracking set.
    LazyPopUniquePriorityQueue::pop(): Returns the K-SV pair with the highest SV whose key is not pending deletion.
    LazyPopUniquePriorityQueue::remove(): Removes an element from the priority queue immediately.
    LazyPopUniquePriorityQueue::remove_at_pop_time(): Removes an element from the priority queue when it is popped.
    LazyPopUniquePriorityQueue::empty(): Returns whether the queue is empty.
    LazyPopUniquePriorityQueue::clear(): Clears the priority queue vector and the tracking sets.
    LazyPopUniquePriorityQueue::size(): Returns the number of elements in the queue.
    LazyPopUniquePriorityQueue::contains(): Returns true if the key is in the queue, false otherwise.

Related Issue

This PR tries to resolve this issue: #2859

Motivation and Context

The original pack uses a fixed size vector to store the proposed candiates, and the candiates are sorted using insertion sort. If we want to increase the size, this data structure be less efficient than a priority queue.

How Has This Been Tested?

It has been tested using Titan_quick_qor tests. It shows slight worse CPD, 1% increase, and better WL, 1% decrease. 3% reduction in pack time, and 11% reduction in placement time.
image

Types of changes

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • All new and existing tests passed

@github-actions github-actions bot added VPR VPR FPGA Placement & Routing Tool lang-cpp C/C++ code labels Apr 22, 2025
@RonZ13 RonZ13 marked this pull request as ready for review April 23, 2025 17:24
@RonZ13 RonZ13 force-pushed the packer-feasible-candidates-list-to-priority-queue branch from 320571a to 79e6af8 Compare April 23, 2025 17:45
Copy link
Contributor

@AlexandreSinger AlexandreSinger left a comment

Choose a reason for hiding this comment

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

Overall a very welcome change, thank you!

I left some comments. I am a little concerned about how you tell if the heap is empty or not. Just getting the size of the heap may not include any elements which have been marked for deletion.

@RonZ13 RonZ13 force-pushed the packer-feasible-candidates-list-to-priority-queue branch 3 times, most recently from 0e7af78 to 8de3926 Compare May 2, 2025 19:57
@RonZ13 RonZ13 requested a review from AlexandreSinger May 6, 2025 16:02
@RonZ13 RonZ13 force-pushed the packer-feasible-candidates-list-to-priority-queue branch 2 times, most recently from 304df48 to 60575e9 Compare May 6, 2025 19:41
@AlexandreSinger
Copy link
Contributor

@RonZ13 These changes look good to me. We need to run the Nightly Tests on this branch before merging. I cannot run this PR on the NightlyTestManual runner from github due to this branch being on your private fork (we can only run it on branches on Master). I have a feeling that the Nightly Tests will also have some failures...

@RonZ13
Copy link
Author

RonZ13 commented May 7, 2025

@RonZ13 These changes look good to me. We need to run the Nightly Tests on this branch before merging. I cannot run this PR on the NightlyTestManual runner from github due to this branch being on your private fork (we can only run it on branches on Master). I have a feeling that the Nightly Tests will also have some failures...

@AlexandreSinger I am a bit confused. Do I run the Nightly test locally? or do I merge this packer-feasible-candidate-queue branch into master branch in my forked repo, then the CI will automatically run on this PR?

@AlexandreSinger
Copy link
Contributor

@RonZ13 These changes look good to me. We need to run the Nightly Tests on this branch before merging. I cannot run this PR on the NightlyTestManual runner from github due to this branch being on your private fork (we can only run it on branches on Master). I have a feeling that the Nightly Tests will also have some failures...

@AlexandreSinger I am a bit confused. Do I run the Nightly test locally? or do I merge this packer-feasible-candidate-queue branch into master branch in my forked repo, then the CI will automatically run on this PR?

The CI is run weekly on Sundays automatically only on Sundays. I do not recommend merging your code in and then waiting for that event to happen. You can run the Nightly Tests locally on Wintermute (you will need many cores to run any of the Nightly Tests in a reasonable amount of time).

Another option is to use the interface that @AmirhosseinPoolad made: https://docs.verilogtorouting.org/en/latest/README.developers/#manual-nightly-tests
However, this approach requires the branch you want to run to be in the VTR repository (not on a fork). You can raise another branch on origin with the same code and run it using the instructions above. Regardless we need the NightlyTests ran (and most likely the golden results generated) before merging this PR. If we do not do this, the NightlyTests will fail on Master which is very nasty to fix after the PR has been merged.

Rongbo Zhang and others added 3 commits May 12, 2025 15:00
…ePriorityQueue.

    The class LazyPopUniquePriorityQueue is a priority queue that allows for lazy deletion of elements.
    It is implemented using a vector and 2 sets, one set keeps track of the elements in the queue, and the other set keeps track of the elements that are pending deletion.
    The queue is sorted by the sort-value(SV) of the elements, and the elements are stored in a vector.
    The set is used to keep track of the elements that are pending deletion, so that they can be removed from the queue when they are popped.
    The class definiation can be found in vpr/src/util/lazy_pop_unique_priority_queue.h

    Currently, the class supports the following functions:
        LazyPopUniquePriorityQueue::push(): Pushes a key-sort-value (K-SV) pair into the priority queue and adds the key to the tracking set.
        LazyPopUniquePriorityQueue::pop(): Returns the K-SV pair with the highest SV whose key is not pending deletion.
        LazyPopUniquePriorityQueue::remove(): Removes an element from the priority queue immediately.
        LazyPopUniquePriorityQueue::remove_at_pop_time(): Removes an element from the priority queue when it is popped.
        LazyPopUniquePriorityQueue::empty(): Returns whether the queue is empty.
        LazyPopUniquePriorityQueue::clear(): Clears the priority queue vector and the tracking sets.
        LazyPopUniquePriorityQueue::size(): Returns the number of elements in the queue.
        LazyPopUniquePriorityQueue::contains(): Returns true if the key is in the queue, false otherwise.
@RonZ13 RonZ13 force-pushed the packer-feasible-candidates-list-to-priority-queue branch from 0d8934f to e0a1fad Compare May 12, 2025 19:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang-cpp C/C++ code VPR VPR FPGA Placement & Routing Tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants