perf: replace per-year pandas scalar access with numpy arrays in run loop - #216
Merged
Conversation
emi2conc, conc2forc, methane_lifetime and calculate_forc_three_main are called once per tracer per year, and each repeatedly indexed df_gas by column+row (e.g. df_gas["ALPHA"][tracer]), which is expensive pandas scalar access. Build plain-dict caches of the static df_gas columns once per run in _build_df_gas_cache() and read from those instead. Also replaces the per-step df_gas.at[...] NAT_EM writes for CH4/N2O with a local nat_em variable, removing repeated DataFrame mutation. Results are unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Per-year, per-tracer code indexed self.emis (a pandas DataFrame) by column+year label, and calc_aerosol_forcing rebuilt its reference-species dict on every call. Cache emis columns as numpy arrays once per run and index them positionally via yr - nystart, and build the aerosol reference map once in _build_df_gas_cache(). Applies to calc_aerosol_forcing, tropospheric_ozone_forcing, methane_lifetime and emi2conc. The emis index is contiguous (nystart..nyend), so positional indexing is equivalent to the prior label lookups; results are unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The year loop in _run and add_year_data_to_output indexed the solar, volcanic and land-use-change forcing via pandas .iloc every year (~7 scalar .iloc accesses per year), which dominated the remaining pandas overhead in the hot path. These inputs are static for a run, so convert them to numpy arrays once in __init__ and index positionally by yr - nystart. The volcanic NH/SH per-year means are also precomputed. The source DataFrames are kept for the rf_run path; results are unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Apply black line-wrapping to the cached hot-path expressions and declare the hot-path cache attributes in __init__ so pylint no longer reports attribute-defined-outside-init. Output remains bit-identical. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
maritsandstad
approved these changes
Jun 3, 2026
maritsandstad
left a comment
maritsandstad
left a comment
Contributor
There was a problem hiding this comment.
Maybe this is all ok like this, CHANGELOG entry though...
Comment on lines
+141
to
+149
| # Cache solar/volcanic/LUC forcing as numpy arrays for the year loop; | ||
| # these inputs are static for the run, so positional indexing by | ||
| # (yr - nystart) avoids repeated pandas .iloc access in the hot path. | ||
| self._sun_arr = self.rf_volc_sun["sun"].iloc[:, 0].to_numpy() | ||
| self._volc_n_arr = self.rf_volc_sun["volc_n"].to_numpy() | ||
| self._volc_s_arr = self.rf_volc_sun["volc_s"].to_numpy() | ||
| self._volc_n_mean = self._volc_n_arr.mean(axis=1) | ||
| self._volc_s_mean = self._volc_s_arr.mean(axis=1) | ||
| self._rf_luc_arr = self.rf_luc.iloc[:, 0].to_numpy() |
Contributor
There was a problem hiding this comment.
If this is what we are going with, I suggest letting a method set all of these from the output of the input handler and not keeping a self.rf_luc or self.rf_volc_sun etc be part of the object at all...
Contributor
There was a problem hiding this comment.
This one I might actually want to change though...
Comment on lines
+395
to
+403
| self._alpha = self.df_gas["ALPHA"].to_dict() | ||
| self._sarf_to_erf = self.df_gas["SARF_TO_ERF"].to_dict() | ||
| self._conc_unit = self.df_gas["CONC_UNIT"].to_dict() | ||
| self._tau1 = self.df_gas["TAU1"].to_dict() | ||
| self._beta = self.df_gas["BETA"].to_dict() | ||
| self._nat_em = self.df_gas["NAT_EM"].to_dict() | ||
| self._nat_em_co2 = float(self.df_gas["NAT_EM"]["CO2"]) | ||
| self._inv_tau2_ch4 = 1.0 / float(self.df_gas["TAU2"]["CH4"]) | ||
| self._inv_tau3_ch4 = 1.0 / float(self.df_gas["TAU3"]["CH4"]) |
Contributor
There was a problem hiding this comment.
Can we make this a nested dict instead.
Contributor
There was a problem hiding this comment.
Or maybe it's ok, but perhaps having a separate one for CO2 natural emissions is overkill?
Contributor
There was a problem hiding this comment.
It's also partially assuming TAU2 and TAU3 will never be used for anything but methane, but maybe that's fine...
Pushes the pandas->numpy conversion into ``read_data_on_year_row`` so the DataFrame layer that was being constructed and immediately discarded by ``CICEROSCM.__init__`` (Marit's TODO from 980ba22) goes away. The file path now reads via ``np.loadtxt``; the ``_data`` branch of ``get_data`` normalises any user-supplied DataFrame to the same shape contract (1D for sun/luc, 2D for volc). All seven SSP245 outputs remain byte-identical to the PR tip and the 113-test suite still passes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
One logical idea applied across the run loop: in the hot per-year path we were repeatedly doing pandas
DataFrame/Seriesscalar lookups (df[col][yr]), which carry significant per-access overhead. Cache the relevant columns as numpy arrays once and index them positionally. All changes are bit-identical — full SSP245 1750–2100 output matchesmainbyte-for-byte; full suite (113 tests) passes.Three commits:
df_gascolumn lookups in the concentrations/emissions hot path — precompute per-gas scalar parameters (alpha, tau, beta, natural-emission terms, etc.) into arrays/dicts instead of re-reading DataFrame cells each year.yr - yr0.ciceroscm.py_runandadd_year_data_to_outputread cached arrays instead of.ilocinto the source DataFrames (DataFrames retained for therf_runpath).Measured impact (SSP245 → 2100,
_runwall-clock, 5 runs)_runmean(*measured in the full speedup chain after an unrelated thermal commit; the three changes here account for ~0.45s / ~31% of baseline run time.)
Test plan
pytestfull suite — 113 passedmain(all 7 SSP245 output files, byte-compared)years[0]for non-defaultnystart/nyend🤖 Generated with Claude Code