Skip to content

Conversation

@ozhanozen
Copy link
Contributor

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 #2759.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist

  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@ooctipus
Copy link
Collaborator

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.

@ozhanozen
Copy link
Contributor Author

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:

if self._feature_dim is None:
   *_, self._feature_dim = data.shape
   self._bias = self._bias.repeat(1, self._feature_dim)
   self.reset()

only ever happens once, on the very first apply. At that moment:
1. self._feature_dim goes from None → some integer.
2. We expand the bias from shape (N,1) → (N,D).
3. We invoke self.reset(), which re-samples that full (N,D) bias in place.

After that first call, self._feature_dim is no longer None, so we never enter that branch again. apply jumps straight to return super().apply(data) + self._bias. Subsequent steps are as they were before. No extra calls to reset() during an episode: no per-step re-sampling.

Copy link
Collaborator

@ooctipus ooctipus left a 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!

@kellyguo11
Copy link
Contributor

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?

@ozhanozen
Copy link
Contributor Author

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 sample_bias_per_component flag (default True) to enable/disable backwards compatibility, updated the PR with the latest changes, and updated extension version & changelog. PR is ready to be merged from my side.

@kellyguo11 kellyguo11 merged commit 1ad83e1 into isaac-sim:main Jun 26, 2025
4 of 5 checks passed
@ozhanozen ozhanozen deleted the fix/noise-bias-shape branch June 27, 2025 15:06
harry-wf-cheung pushed a commit to harry-wf-cheung/IsaacLab-Harry that referenced this pull request Jul 30, 2025
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Proposal] Add per-feature bias sampling in NoiseModelWithAdditiveBias

3 participants