[BE] Fix the env pause issue

[jimmytyyang]

Motivation and Context

This is to fix the issue when doing an evaluation on the HRL policy, the size of internal stateful parameters does not match the number of active skills. The issue has been known in multiple places:

(1) our team's slack channel
(2) github issues: #1718
(3) myself

How Has This Been Tested

Loading the social nav checkpoint and run full dataset evaluation

Types of changes

• [Bug Fix] (non-breaking change which fixes an issue)

Checklist

• My code follows the code style of this project.
• I have updated the documentation if required.
• I have read the CONTRIBUTING document.
• I have completed my CLA (see CONTRIBUTING)
• I have added tests to cover my changes if required.

[ASzot]

Why add this? Should on_envs_pause should be able to handle empty lists of ends to pause?

[xavierpuigf]

Same comment as Andrew, I think you dont need the if condition and can always call envs_to_pause with empty list. Unless the reason is efficiecny in not doing these checks.
Thanks for the fix!

[jimmytyyang]

@xavierpuigf and @ASzot Thanks for the review! I noticed that this code (

habitat-lab/habitat-baselines/habitat_baselines/rl/hrl/hl/fixed_policy.py

Line 38 in 379b1de

if len(solution_actions) == 0:

) could prevent us from removing the condition if any(envs_to_pause):

Maybe we could add a check inside def filter_envs(self, curr_envs_to_keep_active)?

[xavierpuigf]

@xavierpuigf and @ASzot Thanks for the review! I noticed that this code (

habitat-lab/habitat-baselines/habitat_baselines/rl/hrl/hl/fixed_policy.py

Line 38 in 379b1de

if len(solution_actions) == 0:

) could prevent us from removing the condition if any(envs_to_pause):
Maybe we could add a check inside def filter_envs(self, curr_envs_to_keep_active)?

good point! yeah I would add the check in fixed_policy instead, as you suggest. @jimmytyyang

[jimmytyyang]

Thanks for the review! Andrew spoke with me saying that he is fine with the current implementation, and it might introduce an overhead if there are no envs to pause. So based on this and what Xavi suggested, I feel that I would like to expose this and I will add a comment there to say why we want to have this condition. Thanks for the awesome review! :)