Skip to content

ENH: add num_interp to AlignedSpin prior#912

Merged
mj-will merged 4 commits intobilby-dev:mainfrom
mj-will:aligned-spin-add-num-interp
Feb 13, 2025
Merged

ENH: add num_interp to AlignedSpin prior#912
mj-will merged 4 commits intobilby-dev:mainfrom
mj-will:aligned-spin-add-num-interp

Conversation

@mj-will
Copy link
Copy Markdown
Collaborator

@mj-will mj-will commented Feb 11, 2025

#849 added a new test to the prior_test.py but this test is currently taking ~1 hour total in the CI. To address this, I've added num_interp to the AlignedSpin prior.

This allows us to speed up the test but allows the user to configure the interpolation. The defaults are the same (different for each case).

On my laptop, this reduced the time from ~20 minutes to ~ 2 minutes, but we'll see how the CI behaves.

@adivijaykumar
Copy link
Copy Markdown
Collaborator

I haven't thought too much about this, but will the default of 10_000 and 100_000 also make sampling slower? We want to avoid that too, right?

@mj-will
Copy link
Copy Markdown
Collaborator Author

mj-will commented Feb 13, 2025

I haven't thought too much about this, but will the default of 10_000 and 100_000 also make sampling slower? We want to avoid that too, right?

So neither should affect sampling, it should be a one-time cost. Even with 100,000 the simple method seems to be quick enough that it doesn't slow down the tests.

Though perhaps it would be good to document these choices, @ColmTalbot was there a particular motivation behind these choices?

@ColmTalbot
Copy link
Copy Markdown
Collaborator

I'm fairly sure the original reason for bumping up the number is accuracy close to the peak. The density diverges at zero and so to resolve this well we need a lot of points. I think this is a good compromise.

@mj-will
Copy link
Copy Markdown
Collaborator Author

mj-will commented Feb 13, 2025

Thanks both, I'll probably add this in to 2.5.0 just to make preparing the release slightly quicker.

@mj-will mj-will added this to the 2.5.0 milestone Feb 13, 2025
@mj-will mj-will merged commit 534062e into bilby-dev:main Feb 13, 2025
@mj-will mj-will deleted the aligned-spin-add-num-interp branch February 13, 2025 14:08
lorenzopompili00 pushed a commit to lorenzopompili00/bilby that referenced this pull request Jun 30, 2025
* ENH: add `num_interp` to `AlignedSpin` prior

* TST: specify `num_interp` for aligned spin prior

* MAINT: make `num_interp` an attribute

* BUG: fix `num_interp`
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.

3 participants