Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix trajectory sampling to handle active dims #790

Merged
merged 3 commits into from
Oct 16, 2023
Merged

Fix trajectory sampling to handle active dims #790

merged 3 commits into from
Oct 16, 2023

Conversation

khurram-ghani
Copy link
Collaborator

@khurram-ghani khurram-ghani commented Oct 13, 2023 •
edited
Loading

Related issue(s)/PRs: None

Summary

Fixes issue where trajectory sampling was ignoring active-dims in the kernel

Fully backwards compatible: yes

PR checklist

  • The quality checks are all passing
  • The bug case / new feature is covered by tests
  • Any new features are well-documented (in docstrings or notebooks)

@khurram-ghani khurram-ghani requested a review from hstojic October 13, 2023 20:15
@hstojic hstojic requested review from alansaul and vpicheny October 13, 2023 20:27
hstojic
hstojic reviewed Oct 13, 2023
View reviewed changes
Copy link
Collaborator

@hstojic hstojic left a comment

Choose a reason for hiding this comment

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

this is great, thanks!
but we should also add a test for active_dims, to make sure we would catch smth like this

@khurram-ghani khurram-ghani merged commit aa94c86 into develop Oct 16, 2023
12 checks passed
@khurram-ghani khurram-ghani deleted the khurram/sampler_active_dims_fix branch October 16, 2023 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hstojic hstojic hstojic approved these changes

@alansaul alansaul Awaiting requested review from alansaul

@vpicheny vpicheny Awaiting requested review from vpicheny

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@khurram-ghani @hstojic