• ooctipus's avatar
    Separates per-step termination and last-episode termination bookkeeping (#3745) · f4982455
    ooctipus authored
    # Description
    
    This PR fixes the issue where get_done_term returned last episode value
    rather than current step value.
    
    This PR realizes values used for get_term should be different from that
    used for logging, and mixed useage leads to non-intuitive behavior.
    
    using per-step value for logging leads to overcounting and undercounting
    reported in #2977
    using last-episode value for get_term leads to misalignment with
    expectation reported in #3720
    
    Fixes #2977 #3720 
    
    ---
    
    The logging behavior remains *mostly* the same as #3107, and and also
    got rid of the weird overwriting behavior(yay).
    I get exactly the same termination curve as #3107 when run on
    `Isaac-Velocity-Rough-Anymal-C-v0`
    
    Here is a benchmark summary with 1000 steps running
    `Isaac-Velocity-Rough-Anymal-C-v0 ` with 4096 envs
    
    Before #3107: 
    
    `| termination.compute     |       0.229 ms|`
    `| termination.reset       |       0.007 ms|`
    
    PR #3107: 
    
    `| termination.compute     |       0.274 ms|`
    `| termination.reset       |       0.004 ms|`
    
    This PR: 
    
    `| termination.compute     |       0.258 ms|`
    `| termination.reset       |       0.004 ms|`
    
    We actually see improvement, this is due to the fact that expensive
    maintenance of last_episode_value is only computed once per
    compute(#3107 computes last_episode_value for every term)
    
    ## Type of change
    
    - Bug fix (non-breaking change which fixes an issue)
    
    ## Checklist
    
    - [x] I have read and understood the [contribution
    guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html)
    - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
    `./isaaclab.sh --format`
    - [ ] I have made corresponding changes to the documentation
    - [x] My changes generate no new warnings
    - [ ] I have added tests that prove my fix is effective or that my
    feature works
    - [x] I have updated the changelog and the corresponding version in the
    extension's `config/extension.toml` file
    - [x] I have added my name to the `CONTRIBUTORS.md` or my name already
    exists there
    
    ---------
    Signed-off-by: 's avatarKelly Guo <kellyg@nvidia.com>
    Co-authored-by: 's avatarKelly Guo <kellyg@nvidia.com>
    f4982455