Changed the random number generator in packer to deterministic vtr::irand #2871
Conversation
github-actions
bot
added
VPR
There was a problem hiding this comment.
@haydar-c Thank you for this change! I have some style comments that I think would improve your change and improve the documentation.
vpr/src/pack/greedy_clusterer.cpp
Outdated
| @@ -160,6 +161,8 @@ GreedyClusterer::do_clustering(ClusterLegalizer& cluster_legalizer, | |||
|
|
|||
| print_pack_status_header(); | |||
|
|
|||
| vtr::RngContainer rng(0); | |||
There was a problem hiding this comment.
I guess that the only user of this RngContainer object is GreedyCandidateSelector. If this is the case, I think it makes more sense to add it as a member variable to GreedyCandidateSelector and initilize it in its constructor instead of passing it down a deep function call hierarchy.
There was a problem hiding this comment.
I agree! That would make things much cleaner! Especially since it is an independent object!
…c vtr::irand" This reverts commit 53cd616.
There was a problem hiding this comment.
@haydar-c This is a beautiful PR now! I really like this implementation! Great job. LGTM
Thanks for the review! Merging this now. |
|
@haydar-c Perhaps wait to merge this until Vaughn has a quick look! Edit: He may have other comments than mine. |
Sure, still learning the conventions. Thanks for pointing that out. |
| @@ -516,5 +517,9 @@ class GreedyCandidateSelector { | |||
| /// @brief A count on the number of unrelated clustering attempts which | |||
| /// have been performed. | |||
| int num_unrelated_clustering_attempts_ = 0; | |||
|
|
|||
| /// @brief Random number generator to get a random atom between 0 and | |||
There was a problem hiding this comment.
I'd make this slightly more general to start.
I'd change this to:
@brief Random number generator used by the clusterer. Currently this is used only when selecting atoms from
attraction groups, but could be used for other purposes in the future.
|
LGTM. Merging. |
Description
Changed the random number generator in add_cluster_molecule_candidates_by_attraction_group in packer to be vtr::irand. Initiated the vtr::RngContainer in do_clustering before each reference of try_glow_cluster.
Related Issue
#2858
[Pack] Random Number Generator in Packer is Non-Deterministic #2858
Motivation and Context
Aim is to make the adding cluster molecule candidate by attraction groups in packing deterministic.
How Has This Been Tested?
Local tests: vtr_reg_basic, vtr_reg_strong
CI Test Workflow
Types of changes
Checklist: