-
Notifications
You must be signed in to change notification settings - Fork 3k
Updates NoiseModelWithAdditiveBias to apply per-feature bias sampling
#2760
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
Conversation
|
Thank you for PR, this is a very nice feature to have. One small question as I review the code, if you relie on self.reset() to re-sample that expanded bias in-place, does that mean multi-dim intput will have a different bias value every step? My intuition tells me the bias per feature should at least stay consistent throughout the episode and reset at next episode, If I am not wrong, the code is currently resample-per-episode for 1D bias, yet resample-per-step for N-D bias. |
Hi @ooctipus, The code section I wrote: only ever happens once, on the very first After that first call, |
ooctipus
left a comment
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.
Ahhh, I see. Thanks for the clarification, I think the code looks good and clean! Good Job!
|
I think having an option to enable the backwards compatible approach will still be useful. Do you mind rebasing the changes on top of the other PR for noise model changes and update the extension versions? |
I have added the |
…ng (isaac-sim#2760) # Description This PR updates the `NoiseModelWithAdditiveBias` to apply per-feature bias sampling. Previously, the model sampled a single scalar bias per episode and applied it uniformly across all feature dimensions (i.e., axis 1 of a (`num_env`, `feature_dim`) tensor). This PR changes the behavior to instead sample a separate bias value for each feature dimension, making the model more suitable for structured inputs such as positions, velocities, or multi-DOF actions. ### Notes Structured inputs typically contain semantically distinct components, like [x, y, z] coordinates, where applying the same bias across all components introduces unrealistic, fully correlated noise. Independent per-dimension bias sampling leads to more realistic and robust policy training, especially for sim-to-real transfer. I’ve replaced the previous behavior with this new default, as I believe the original implementation could be misleading and not well-suited for many practical scenarios. However, if desired, I can make both behaviors available via a configuration flag (e.g., `per_feature_bias=True`), to retain backward compatibility. Fixes isaac-sim#2759. ## Type of change - Bug fix (non-breaking change which fixes an issue) - New feature (non-breaking change which adds functionality) ## Checklist - [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 --------- Co-authored-by: Kelly Guo <kellyg@nvidia.com>
Description
This PR updates the
NoiseModelWithAdditiveBiasto apply per-feature bias sampling.Previously, the model sampled a single scalar bias per episode and applied it uniformly across all feature dimensions (i.e., axis 1 of a (
num_env,feature_dim) tensor). This PR changes the behavior to instead sample a separate bias value for each feature dimension, making the model more suitable for structured inputs such as positions, velocities, or multi-DOF actions.Notes
Structured inputs typically contain semantically distinct components, like [x, y, z] coordinates, where applying the same bias across all components introduces unrealistic, fully correlated noise. Independent per-dimension bias sampling leads to more realistic and robust policy training, especially for sim-to-real transfer.
I’ve replaced the previous behavior with this new default, as I believe the original implementation could be misleading and not well-suited for many practical scenarios. However, if desired, I can make both behaviors available via a configuration flag (e.g.,
per_feature_bias=True), to retain backward compatibility.Fixes #2759.
Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there