API restoration #1083
API restoration #1083
Conversation
| @@ -694,7 +694,7 @@ def _get_halo_model_nonlin_power(self): | |||
| prf = hal.HaloProfileNFW(concentration=c) | |||
| hmc = hal.HMCalculator(mass_function=hmf, halo_bias=hbf, | |||
| mass_def=mdef) | |||
| return hal.halomod_Pk2D(self, hmc, prf) | |||
| return hal.halomod_Pk2D(self, hmc, prf, normprof1=True) | |||
There was a problem hiding this comment.
without this, the halo model power spectrum as a Cosmology power spectrum option (which will be deprecated in v3) gave the wrong answer.
| if len(out) != 3: | ||
| raise ValueError("A valid mass function and halo bias is " | ||
| "needed") | ||
| self.mass_def, self.mass_function, self.halo_bias = out |
There was a problem hiding this comment.
This is just here to throw an actually useful error if any of these arguments are set to None (the corresponding python-level error was extremely obscure)
| warnings.warn("In v3 all profiles will need an associated " | ||
| "mass definition.", CCLDeprecationWarning) | ||
| prof.mass_def = self.mass_def | ||
|
|
There was a problem hiding this comment.
This was necessary in order to ensure that we could use profiles that were instantiated without a mass definition attached to them from scratch (this will not be possible in v3, but it was in v2).
| bA = i11_A / nA if isinstance(pA, ProfNC) else np.zeros_like(k_use) | ||
| bB = i11_B / nB if isinstance(pB, ProfNC) else np.zeros_like(k_use) | ||
| bA = i11_A / nA if pA.is_number_counts else np.zeros_like(k_use) | ||
| bB = i11_B / nB if pB.is_number_counts else np.zeros_like(k_use) |
There was a problem hiding this comment.
Using profile class type broke the API.
| "``None`` will be deprecated in future versions.", | ||
| CCLDeprecationWarning) | ||
| name = DEFAULT_POWER_SPECTRUM | ||
| elif isinstance(p_of_k_a, str): |
There was a problem hiding this comment.
This was to throw an actually useful warning.
| if (prof, prof2, prof12_2pt) == (prof3, prof4, prof34_2pt): | ||
| dpk34[ia] -= dpk12[ia] | ||
| dpk34[ia] = dpk12[ia] |
There was a problem hiding this comment.
this was a huge bug
| assert issubclass(w[0].category, wtype), \ | ||
| "Warning raised was the wrong type (got %s, expected %s)" % ( | ||
| w[0].category, wtype) | ||
| return res |
There was a problem hiding this comment.
formally speaking, this was part of the API, even though we don't really use it.
| @@ -128,3 +130,119 @@ def test_tkkssc_linear_bias_raises(): | |||
| """Test that it raises if the profile is not NFW.""" | |||
| with pytest.raises(TypeError): | |||
| ccl.halos.halomod_Tk3D_SSC_linear_bias(COSMO, HMC, prof=GNFW) | |||
|
|
|||
There was a problem hiding this comment.
This restores a test that was lost in the last few PRs, but which is quite useful (it actually uncovered the bug mentioned above)
Pull Request Test Coverage Report for Build 4925502085
💛 - Coveralls |
The current version of the code implements the v3 API while attempting to preserve the v2 API. In this PR, I made a copy of the unit tests and benchmarks (not part of the PR) as they were before any of these modifications took place (in the latest released version, v2.7). I then ran the tests, which use the pure v2 API, and ensured that they pass.
In doing so, I weeded out a couple of bugs and restored some pieces of code that strictly speaking are part of the v2 API.
Now the caveats. Not all old tests passed, but the reasons behind those that didn't pass are all reasonable:
MassFunctions ,HaloBiass etc. cannot be instantiated anymore. It is doubtful that anyone was using them (as they are completely useless), so I think it's worth living with it as a very mild API breakage.Cosmologys hold now always holds the right number of masses (before it had a 3-element array, regardless of how many massive neutrinos there were).Nonenow have a default value. In all cases the default value attempts to mimic the behaviour of it beingNonein the old API.I'll explain some of the changes as comments below.