-
Notifications
You must be signed in to change notification settings - Fork 412
[APPack] Updated Max Candidate Distance Interface #3021
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
[APPack] Updated Max Candidate Distance Interface #3021
Conversation
3fe979c
to
a77545a
Compare
a77545a
to
1a5b841
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good Alex, thanks! Regarding the default scaling values for different block types, I was wondering if you ran an extensive parameter sweep or just tried a bunch of values and found the current ones to work best.
@amin1377 Regarding where the default values came from, I did perform a parameter sweep around 1 month ago when I was tuning the flow for a paper. However, a lot has changed in the last month which will have caused these values to change. I plan to do another sweep later. This interface will make this sweep significantly easier. |
The max candidate distance is used by APPack to decide which molecules to ignore when packing, based on their distance from the cluster being formed. Cleaned up the interface of this by pre-computing the max candidate distance of all logical blocks ahead of time and reading from these pre-computed values during packing. Added a command-line option to allow the user to override some or all of these max distance thresholds. By default, VPR will select values based on the type of logical block and the primitives it contains. Fixed issue with APPack creating too many IO blocks for some circuits due to the max candidate distance thresholds for IO blocks being too low. More tuning should be done on these values once the mass legalizer has been cleaned up a bit more.
1a5b841
to
4079c04
Compare
@amin1377 Thank you so much for the review. I agreed with your comment and updated how the class is initialized. Please let me know if you have any further comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, Alex! Thanks!
The max candidate distance is used by APPack to decide which molecules to ignore when packing, based on their distance from the cluster being formed.
Cleaned up the interface of this by pre-computing the max candidate distance of all logical blocks ahead of time and reading from these pre-computed values during packing.
Added a command-line option to allow the user to override some or all of these max distance thresholds. By default, VPR will select values based on the type of logical block and the primitives it contains.
Fixed issue with APPack creating too many IO blocks for some circuits due to the max candidate distance thresholds for IO blocks being too low.
More tuning should be done on these values once the mass legalizer has been cleaned up a bit more.
Resolves #3020