Repository navigation
Apology for the #211 co-author trailer; fix crashes, silent errors and API input handling (AI agent) - #212
Conversation
…ortran (AI agent) Process-killing cases (ERROR STOP, segfault, heap corruption or hang) that now raise catchable Python exceptions: - mathutils threej/threej_pt with |m| > l - get_background_[redshift|time]_evolution and get_time_evolution after get_background(no_thermo=True) (ThermoData%Init needs InitVars' dtaurec); get_time_evolution with k <= 0, times after tau0, or no background - negative matter power redshifts; min_l other than 1 or 2 - vector modes with massive neutrinos; source windows or custom sources in non-flat models; GaussianSourceWindow sigma <= 0 - SplinedInitialPower used without the needed scalar/tensor table; log-regular tables with < 2 points or kmin >= kmax - dark energy w(a) tables with non-increasing a - ExternalNonLinearRatio with < 2 or non-increasing grid points, or unset - EarlyQuintessence with zc before the start of integration Silent wrong results: - non-linear lensing source scaling spline used transfer_times(0:n-1) as knots for scaling(1:n) (bounds 0:n+1), so the knots were shifted and the first was uninitialised memory, giving all-NaN lensed spectra when it held a NaN and ~5e-5 errors in C_phi at high L otherwise (same fix as cmbant#211, included so this change is complete on its own) - read_ini/write_ini reset the global lensing_method from 5 to 1 when the ini did not set it, changing all later Python calculations - get_zre_from_tau returned the stored redshift (10) for tau < 0.001 - set_initial_power_table(k, pk=None) made Fortran read past an empty PK array; unsorted or non-positive k gave garbage splines - lensing potential C_L scaling factors computed in single precision (~2e-7 relative error); A10 constant single precision - set_H0_for_theta did not apply the final H0 through setter_H0 Other fixes: - scalar_coupling_matrix/pcl_coupling_matrix failed for mask spectra longer than 2*lmax+1; chi_squared accepts lists and non C-ordered arrays - write_ini writes all AccuracyParams boosts read by camb.f90 and vector_mode - read_ini from URL raises on HTTP errors and always removes its temp file - IniFile.original_filename is the top-level file, not the last INCLUDE - symbolic compiles in a private temp directory (no shared names/chdir) - clearer errors for unknown named-int values and string booleans - list/integer inputs accepted by distance, density, Omega, dark energy, lensing, BAO, source window and external ratio functions - get_BAO params optional as documented; unknown dotted set_params names raise CAMBUnknownArgumentError; CAMBparams.diff aligns differing lines - SP(k) warning string spacing; docstring and params.ini comment fixes
- CheckSupportedParams in CAMBdata_SetParams sets a global error for min_l, negative matter power redshifts, vectors with massive neutrinos or non-flat (previously a process stop), source windows/custom sources in non-flat models and Gaussian window sigma <= 0; remove the duplicated Python _check_transfer_params - reset global_error_flag in GetBackgroundThermalEvolution; check thermo data before swapping custom sources in CAMB_TimeEvolution - write_ini: derive accuracy boost names from AccuracyParams fields and only write non-default values; read magnetic flag via config; fix vector-only round trip (DoLensing and tensor parameters) - symbolic: load the DLL inside the try and clean up with rmtree Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Code review of this PR's commits turned up a few issues to tighten up before merging: - BoolField.__set__ now silently coerces any truthy int (e.g. 5) to True instead of raising, narrowing the validation that used to catch most out-of-range values. - get_partially_lensed_cls silently truncated an oversized Alens array instead of raising, where it used to error on a shape mismatch. - set_initial_power_table(k) with neither pk nor pk_tensor silently built an empty InitPower table instead of failing at the call site. - scalar_coupling_matrix([], lmax) raised a raw IndexError rather than a clear validation error. - The k/z/a strictly-increasing + positive checks added by this PR were each written independently in initialpower.py, nonlinear.py and dark_energy.py, with slightly different bundling and messages; factored into one check_strictly_increasing() helper in baseconfig.py (also fixes dark_energy's set_w_a_table crashing with an IndexError, rather than raising cleanly, on an empty table). - get_dark_energy_rho_w(a) and comoving_radial_distance(z) didn't coerce a scalar/array input to float64 as consistently as their siblings. - The two new "thermal history not calculated" guards in camb_python.f90 were duplicated almost verbatim; factored into a shared RequireThermoData() function. - write_ini's new vector_mode output reads Fortran module state (config._magnetic) that isn't actually part of CAMBparams, so it can be stale if a different vector-mode ini was read earlier in the same process; documented the limitation and strengthened the round-trip test to check it. Ran camb.tests.camb_test, mathutils_test and spk_test (33 tests) and pre-commit after these changes.
|
Many thanks for your PR. Claude made two review iterations on it, I 've not reviewed the code yet. |
|
Many thanks for the review and the helpful feedback. I really appreciate the suggestions and will keep them in mind for future PRs. |
The positivity check only needs the first element once the array is known to be increasing, and 'not all(diff > 0)' also catches NaN, which 'any(diff <= 0)' let through.
|
Thanks for the PR, wil merge now! |
|
Thank you very much for merging my PR. I really appreciate your time and the opportunity to contribute to this project. I've also starred forformat, happy to support your work. Thanks again for maintaining such a valuable scientific tool. |
Note about #211 and an apology
First, an apology. The commit merged from #211 (56a95f7, "Fix off-by-one abscissae in non-linear lensing source scaling spline") was prepared with an AI agent, and its message still ends with a co-author trailer naming that AI agent. Because of that trailer GitHub now lists the AI agent as a contributor to CAMB. This does not follow the AI agent guidelines in CONTRIBUTING.md, and I should have removed it before opening that PR. I'm sorry for the trouble.
If you would prefer the AI agent not to appear as a contributor, you could force-push
masterback to the previous version, before 56a95f7. This PR also contains the #211 spline fix itself, without the co-author trailer, so after that you can merge this PR to get the #211 fix back together with the fixes described below. It's entirely your call, and if you'd rather leave history as it is, that's completely fine too. The commit in this PR has no co-author trailer and uses the "AI agent" author name and commit-message tag as described in CONTRIBUTING.md.Summary
This PR fixes bugs found while reviewing the Python wrapper and parts of the Fortran code with an AI agent. Each bug was reproduced before it was fixed, and regression tests were added for most of them. Everything is in a single commit.
Settings that killed the Python process
These cases used to end the whole Python process through
ERROR STOP, a segfault, heap corruption or a hang, so they could not be caught and would kill e.g. a notebook kernel. They now raiseCAMBErrororCAMBValueError:mathutils.threej/threej_ptwith |m| > l.threej_ptis documented to return 0 for these inputs.get_background_redshift_evolution/get_background_time_evolutionandget_time_evolutionafterget_background(..., no_thermo=True). The on-demandThermoData%Initcould never work there, becauseInitVarshad not setdtaurec, soTRanges_Add_deltastopped.get_time_evolutionwith k <= 0, times later thantau0, or before any background has been calculated.min_lother than 1 or 2 (double free).GaussianSourceWindowwithsigma <= 0.SplinedInitialPowerwithout the needed scalar/tensor table. Fortran now raises a global error instead of using an unallocated spline. Also log-regular tables with fewer than two points orkmin >= kmax.ais not increasing.ExternalNonLinearRatiogrids with fewer than two points or non-increasing values, and use beforeset_ratio(now a global error rather thanerror stop).EarlyQuintessencewithzcearlier than the start of integration (1/astart - 1); the minimizer previously hung or crashed.Silent wrong results
State%transfer_timeshas bounds0:num_transfer_redshifts+1, but the whole array was passed as the spline knots forscaling(1:n). The knots were therefore shifted by one, and the first knot was uninitialised memory. If that memory held a NaN, the lensed spectra were returned all-NaN without an error; otherwise C_phi had ~5e-5 errors at high L. This PR includes the fix so that it is complete on its own; it merges cleanly with the currentmaster, which already has it.lensing_methodreset. An ini file withoutlensing_methodset the global lensing method to 1. Soread_ini, andwrite_iniwhich calls it to validate, switched every later Python calculation away from the optimized default (5). This changes BB whenAccurateBB=T. The ini reader now keeps the current value when the key is absent. The sample ini files all setlensing_methodexplicitly, so their results are unchanged.get_zre_from_taufor tau < 0.001. Reionization is switched off below this optical depth, and the function returned the unrelated stored redshift (10 by default). For example tau=0.0009 gave zre=10 while tau=0.005 gave zre=1.1. It now returns 0. Alsoget_zre(tau=0.0)no longer ignores the given tau.set_initial_power_table(k, pk=None)passed an empty P(k) array with n=len(k), so Fortran read beyond the array. Spline tables with unsorted or non-positive k were silently accepted and gave garbage interpolation; they are now rejected. Empty tables still clear the spectrum.real(), giving ~2e-7 relative errors in C_L^phiphi. TheA10constant was also a single-precision literal.set_H0_for_thetadid not apply the final H0 through a customsetter_H0, so model state updated by the setter could be inconsistent with the solved H0.Other fixes
scalar_coupling_matrixandpcl_coupling_matrixfailed with a broadcasting error for mask power spectra longer than2*lmax+1, the usual case.chi_squaredaccepts lists and non-C-ordered or integer arrays.write_ininow writes allAccuracyParamsboosts thatcamb.f90reads, so e.g.TimeStepBoostorLensingBoostno longer fail the round-trip validation. It also writesvector_mode, which is required when reading, so vector-mode parameters previously always failed.read_inifrom a URL raises on HTTP errors and always deletes its temporary file. Arequests.Responseis falsy for error status codes, so the file was leaked.IniFile.original_filenameis now the top-level file rather than the lastINCLUDEd one, sosaveFile()andrelativeFileName()use the right path.symboliccompiles generated code in a private temporary directory with absolute paths. Before, it used shared predictable file names in the system temp directory andos.chdir, so concurrent processes could clash. The gfortran check on Windows now only runs on the first compile, as intended.ValueErrorinstead ofKeyError. Strings assigned to boolean fields raiseTypeErrorinstead ofIndexError; note thatbool("F")isTrue.comoving_radial_distance,redshift_at_comoving_radial_distanceget_background_densities,get_dark_energy_rho_w,get_Omegaget_background_time_evolution, which also fixesget_background_redshift_evolutionfor scalar zget_lensed_cls_with_spectrum,get_lensed_gradient_cls,get_partially_lensed_clsset_dark_energy_w_a,SplinedSourceWindowbias_kz,ExternalNonLinearRatio.set_ratioget_BAO'sparamsargument is now optional, as documented.set_paramsnames raiseCAMBUnknownArgumentError.CAMBparams.diffaligns differing lines, so extra lines in one structure (e.g. source windows) no longer shift all later fields.lensing_methodoption comment inparams.ini.Behaviour changes to be aware of
lensing_methodnow uses the current global value (the optimized default 5) instead of forcing 1. This only changes results for command-line runs withaccurate_BB = Tthat did not setlensing_method.Testing
python -m unittest camb.tests.camb_test: all 22 tests pass, including the new regression tests.python -m unittest camb.tests.mathutils_testandcamb.tests.spk_testpass.python fortran/tests/run_tests.pyagainstCAMB_test_outputs(v2): 136 runs, 0 errors, 0 numerical accuracy mismatches, 0 extra/missing files.pre-commit run --all-filesis clean.master.