Skip to content

feat: Add make_sample() function to WoodburyKernel_varNP and remove params …#122

Merged
meyers-academic merged 1 commit intonanograv:mainfrom
pratyashagitika:fix-woodbury-make-sample
Mar 5, 2026
Merged

feat: Add make_sample() function to WoodburyKernel_varNP and remove params …#122
meyers-academic merged 1 commit intonanograv:mainfrom
pratyashagitika:fix-woodbury-make-sample

Conversation

@pratyashagitika
Copy link
Copy Markdown

…from make_sample() in WoodburyKernel_varN

This PR adds a make_sample() function to the WoodburyKernel_varNP. The issue arose when I was trying to follow the simulations.rst tutorial given within discovery. When running
params = ds.sample_uniform(sampler.params), I got error AttributeError: 'WoodburyKernel_varNP' object has no attribute 'make_sample'.

Further, make_sample(self, params) in WoodburyKernel_varN takes in params argument which differs from all other make_sample() functions in matrix.py. This raises error TypeError: WoodburyKernel_varN.make_sample() missing 1 required positional argument: 'params'. This PR removes params from the argument of make_params in WoodburyKernel_varN.

After changing these two, the simulation code is running successfully.

Please let me know if any modifications are necessary.

@pratyashagitika pratyashagitika changed the title Add make_sample() function to WoodburyKernel_varNP and remove params … feat: Add make_sample() function to WoodburyKernel_varNP and remove params … Feb 24, 2026
@meyers-academic
Copy link
Copy Markdown
Collaborator

Thanks! I will try to look at this before the end of the week. yes, make_sample should probably not take parameters if it's creating a function itself that is used to sample.

For varNP almost certainly no one has had to simulate from that set-up before and so it just wasn't implemented, so thank you for taking the time to do it!

Copy link
Copy Markdown
Collaborator

@meyers-academic meyers-academic left a comment

Choose a reason for hiding this comment

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

This looks great, thank you for catching the bug and for adding this.

@meyers-academic meyers-academic merged commit db8ab2f into nanograv:main Mar 5, 2026
6 of 7 checks passed
@pratyashagitika pratyashagitika deleted the fix-woodbury-make-sample branch April 13, 2026 20:57
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.

2 participants