Skip to content

Conversation

Mayankm96
Copy link
Contributor

@Mayankm96 Mayankm96 commented Jan 30, 2025

Description

Previously on episodic resets, the "time left" for the interval events were not getting resampled. This means that if the user expects the event to happen in the range 6-8s of an episode, it could be that in a new episode, the push happens at a time outside this range as the time left variable keeps its old value.

This MR fixes this issue by resampling the time left inside the manager's reset call.

Note: This only matters for the case when "is_global_time" is False (which is the default). The reason is that when there is global time, the events are triggered through simulation time and not episode time.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

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

@Mayankm96 Mayankm96 changed the title Fix/interval sampling Adds interval resampling on event manager's reset call Jan 30, 2025
@Mayankm96 Mayankm96 requested a review from kellyguo11 as a code owner January 30, 2025 11:39
@Mayankm96 Mayankm96 added the bug Something isn't working label Jan 30, 2025
Copy link
Collaborator

@jsmith-bdai jsmith-bdai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, good find!

@kellyguo11 kellyguo11 merged commit be1e60c into main Jan 30, 2025
5 checks passed
@kellyguo11 kellyguo11 deleted the fix/interval-sampling branch January 30, 2025 18:11
jtigue-bdai pushed a commit that referenced this pull request Apr 14, 2025
# Description

Previously on episodic resets, the "time left" for the interval events
were not getting resampled. This means that if the user expects the
event to happen in the range 6-8s of an episode, it could be that in a
new episode, the push happens at a time outside this range as the time
left variable keeps its old value.

This MR fixes this issue by resampling the time left inside the
manager's reset call.

Note: This only matters for the case when "is_global_time" is False
(which is the default). The reason is that when there is global time,
the events are triggered through simulation time and not episode time.

## Type of change

- Bug fix (non-breaking change which fixes an issue)

## Checklist

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./isaaclab.sh --format`
- [x] 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
SevenFo pushed a commit to SevenFo/IsaacLab that referenced this pull request May 19, 2025
# Description

Previously on episodic resets, the "time left" for the interval events
were not getting resampled. This means that if the user expects the
event to happen in the range 6-8s of an episode, it could be that in a
new episode, the push happens at a time outside this range as the time
left variable keeps its old value.

This MR fixes this issue by resampling the time left inside the
manager's reset call.

Note: This only matters for the case when "is_global_time" is False
(which is the default). The reason is that when there is global time,
the events are triggered through simulation time and not episode time.

## Type of change

- Bug fix (non-breaking change which fixes an issue)

## Checklist

- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./isaaclab.sh --format`
- [x] 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants