[Place] rename get_initial_move_lim to get_place_inner_loop_num_move #2955
Conversation
github-actions
bot
added
VPR
| @@ -50,7 +50,7 @@ t_placer_costs& t_placer_costs::operator+=(const NocCostTerms& noc_delta_cost) { | |||
| return *this; | |||
| } | |||
|
|
|||
| int get_initial_move_lim(const t_placer_opts& placer_opts, const t_annealing_sched& annealing_sched) { | |||
| int get_place_inner_loop_num_move(const t_placer_opts& placer_opts, const t_annealing_sched& annealing_sched) { | |||
There was a problem hiding this comment.
Probably renaming it to comp_place_inner_loop_num_move() makes it clear that it actually computes the value.
There was a problem hiding this comment.
I honestly don't see that much difference between these two names :)
There was a problem hiding this comment.
I think it can go either way. For me, "get_..." methods are usually O(1) lookups. Whereas "calc_..." is used to calculate some value and is usually more expensive. It implicitly says that this function is not as cheap to call and care should be taken when putting it in a loop.
I used the same practice here: https://github.com/verilog-to-routing/vtr-verilog-to-routing/blob/85dfc2983cc74150959aca1ff7666c9b7c292fd7/vpr/src/analytical_place/flat_placement_density_manager.cpp
Although this function is cheap and is O(1), there is some complexity in here that implies that it shouldn't be called many times (For example the log message). Up to you honestly how you want to name this.
There was a problem hiding this comment.
That’s a good way to decide whether to name a function calc_* or get_*. Thanks, Alex!
I think we shouldn’t print the log message inside the helper function. I’ve removed it from there and instead added the log in the annealer constructor where the function is called.
|
@soheilshahrouz: Thanks for reviewing the PR. Feel free to merge it if you don’t have any further suggestions. |
Updated function name from
get_initial_move_limtoget_place_inner_loop_num_move. This clarifies that the number of placement moves within the inner loop is a constant limit, not a variable that changes during annealing.