Remove spline interpolation for pivot approximations#73
Conversation
…, which are pure expansions around the pivot scale, so they do not require interpolation; additionally provide the respective estimates of `n_s` and `n_run` etc. of these approximations
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #73 +/- ##
==========================================
- Coverage 99.09% 99.08% -0.01%
==========================================
Files 19 19
Lines 2439 2415 -24
==========================================
- Hits 2417 2393 -24
Misses 22 22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This pull request removes spline interpolation from the LLMS and ARBDS slow-roll approximation methods for primordial power spectra. The key changes are:
- Direct evaluation: Replaced spline interpolation with direct lambda function evaluation since these approximations are polynomial expansions around the pivot scale
- New attributes: Added method-specific attributes (A_s_LLMS, n_s_LLMS, r_LLMS, etc., and similar for ARBDS3) for the spectral parameters
- Test coverage: Extended tests to verify the new attributes and compare between different approximation methods
- Version bump: Incremented from 2.18.1 to 2.18.2
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| primpy/inflation.py | Removed spline interpolation from LLMS and ARBDS methods; changed function signatures to remove **interp1d_kwargs; added direct lambda functions for power spectra; added new spectral parameter attributes |
| tests/test_inflation.py | Added assertions to test new LLMS and ARBDS3 spectral attributes; added cross-comparison tests between different approximation methods |
| primpy/version.py | Version incremented from 2.18.1 to 2.18.2 |
| README.rst | Documentation version updated to match code version |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # n_s = lambda k: (1 + bs1 | ||
| # + bs2 * np.log(k/K_STAR) | ||
| # + bs3/2 * np.log(k/K_STAR)**2 |
There was a problem hiding this comment.
This commented code appears to be incomplete or leftover from development. It has an incomplete lambda definition without a closing parenthesis and no clear purpose. Consider removing it if it's not needed, or completing it if it serves a documentation purpose.
| # n_s = lambda k: (1 + bs1 | |
| # + bs2 * np.log(k/K_STAR) | |
| # + bs3/2 * np.log(k/K_STAR)**2 |
| sol.logk2logP_t = logk2logP_t_3 | ||
|
|
||
| def derive_approx_power_LLMS(**interp1d_kwargs): | ||
| def derive_approx_power_LLMS(): |
There was a problem hiding this comment.
The function signature was changed to remove **interp1d_kwargs parameter, but the caller at line 874 in derive_approx_power() still passes **interp1d_kwargs. This will cause a TypeError: derive_approx_power_LLMS() got an unexpected keyword argument when method='LLMS' is used.
The same issue exists at lines 1470 and 1524 where **interp_kwargs is passed.
| def derive_approx_power_LLMS(): | |
| def derive_approx_power_LLMS(**interp1d_kwargs): |
| sol.logk2logP_t = logk2logP_t_STE | ||
|
|
||
| def derive_approx_power_ARBDS(order=3, **interp1d_kwargs): | ||
| def derive_approx_power_ARBDS(order=3): |
There was a problem hiding this comment.
The function signature was changed to remove **interp1d_kwargs parameter, but the caller at line 878 in derive_approx_power() still passes **interp1d_kwargs. This will cause a TypeError: derive_approx_power_ARBDS() got an unexpected keyword argument when method='ARBDS' is used.
The same issue exists at lines 1478 and 1532 where **interp_kwargs is passed.
| sol.P_s_approx_ARBDS2 = lambda k: Ps0 * (as0 | ||
| + as1 * np.log(k/K_STAR) | ||
| + as2 * np.log(k/K_STAR)**2) | ||
| sol.P_t_approx_ARBDS2 = lambda k: Pt0 * (at0 |
There was a problem hiding this comment.
Variable shadowing issue: The variables as0, at0, as1, at1, as2, and at2 defined here for order 2 will be overwritten by the order 3 definitions at lines 1362-1367. Since Python lambdas capture variables by reference (late binding), the lambdas sol.P_s_approx_ARBDS2 and sol.P_t_approx_ARBDS2 defined at lines 1354-1359 will incorrectly use the order 3 values when called, not the order 2 values.
This will cause incorrect results when using order=2. To fix this, either use default arguments in the lambda to capture the current values (e.g., lambda k, as0=as0, as1=as1, as2=as2: ...) or use different variable names for each order.
| sol.P_s_approx_ARBDS2 = lambda k: Ps0 * (as0 | |
| + as1 * np.log(k/K_STAR) | |
| + as2 * np.log(k/K_STAR)**2) | |
| sol.P_t_approx_ARBDS2 = lambda k: Pt0 * (at0 | |
| sol.P_s_approx_ARBDS2 = lambda k, as0=as0, as1=as1, as2=as2: Ps0 * (as0 | |
| + as1 * np.log(k/K_STAR) | |
| + as2 * np.log(k/K_STAR)**2) | |
| sol.P_t_approx_ARBDS2 = lambda k, at0=at0, at1=at1, at2=at2: Pt0 * (at0 |
First we remove the spline interpolation of the slow-roll PPS approximation for the LLMS and ARBDS approximations, which do not require interpolation, since they are simply expanded around the pivot scale.
Second, this might lead us to a way of using these types of approximations in situations where the integration fails.
Checklist:
flake8 --max-line-length 99 primpy tests).pydocstyle --convention=numpy primpy).python -m pytest).README.rstandprimpy/__version__.py.