Claude suggested improvements to code quality and speed#15
Open
ChrisJChang wants to merge 4 commits intomainfrom
Open
Claude suggested improvements to code quality and speed#15ChrisJChang wants to merge 4 commits intomainfrom
ChrisJChang wants to merge 4 commits intomainfrom
Conversation
┌───────────────────────────────────────┬──────────┬─────────────────────────────────────────────────────────────────────────────┐ │ Setting │ Default │ Controls │ ├───────────────────────────────────────┼──────────┼─────────────────────────────────────────────────────────────────────────────┤ │ "tick_label_sci_limits" │ (-3, 3) │ Scientific notation threshold for axis tick labels (both 1D and 2D figures) │ ├───────────────────────────────────────┼──────────┼─────────────────────────────────────────────────────────────────────────────┤ │ "colorbar_tick_label_fontsize_offset" │ 3 │ Amount subtracted from fontsize for colorbar tick labels (3 call sites) │ ├───────────────────────────────────────┼──────────┼─────────────────────────────────────────────────────────────────────────────┤ │ "cl_label_x_offset" │ 0.06 │ Fractional x-position of CL/CR text labels along the axes │ ├───────────────────────────────────────┼──────────┼─────────────────────────────────────────────────────────────────────────────┤ │ "cl_label_ha" │ "left" │ Horizontal alignment of CL/CR labels │ ├───────────────────────────────────────┼──────────┼─────────────────────────────────────────────────────────────────────────────┤ │ "cl_label_va" │ "bottom" │ Vertical alignment of CL/CR labels │ ├───────────────────────────────────────┼──────────┼─────────────────────────────────────────────────────────────────────────────┤ │ "profile_linestyle" │ "solid" │ Line style for the 1D profile likelihood / delta chi² curve │ ├───────────────────────────────────────┼──────────┼─────────────────────────────────────────────────────────────────────────────┤ │ "confidence_level_linestyle" │ "dashed" │ Line style for the CL threshold lines in profile/delta chi² plots │ ├───────────────────────────────────────┼──────────┼─────────────────────────────────────────────────────────────────────────────┤ │ "credible_region_linestyle" │ "dashed" │ Line style for the CR threshold lines in 1D posterior plots │ ├───────────────────────────────────────┼──────────┼─────────────────────────────────────────────────────────────────────────────┤ │ "fill_linewidth" │ 0.0 │ Linewidth of fill_between fills (6 call sites) │ ├───────────────────────────────────────┼──────────┼─────────────────────────────────────────────────────────────────────────────┤ │ "separator_linestyle" │ "solid" │ Line style for bin-separator lines in conditional interval plots │ └───────────────────────────────────────┴──────────┴─────────────────────────────────────────────────────────────────────────────┘
#1 — read_hdf5_datasets: eliminate np.append in loop Replaced the per-file np.append pattern (which reallocates the full array on every call) with collecting chunks in a list and calling np.concatenate once at the end. For N files of M points each, this reduces allocations from O(N) full-array copies to one. #2 — fill_nan_with_neighbor_mean: vectorise triple-nested loop Replaced the Python for i / for j / for neighbour loop with scipy.ndimage.generic_filter, which operates in compiled C over the entire array in one pass. #3 — plot_2D_posterior: deduplicate sorted-histogram computation The same np.sort + np.cumsum + normalisation was computed twice in the same function call (once for CR masking, once for contour drawing). Now initialised to None and computed at most once, with the second use reusing the cached result. #4 — Replace deepcopy with shallow copies throughout - Numeric bounds (xy_bounds, x_bounds): replaced with [list(b[0]), list(b[1])] / list(b) — sufficient since only scalars are mutated. - requested_datasets (list of immutable tuples): replaced with list(). - NumPy array slices (y_data[mask], z_data[mask], etc.): removed entirely — fancy indexing already returns a copy. - deepcopy(y_data) in posterior shading: replaced with y_data.copy(). - Removed the now-unused deepcopy import. #5 — plot_1D_profile confidence band loop: single mask computation The boolean mask (x_values > x_start) & (x_values < x_end) was evaluated twice per segment (once for x, once for y) and converted through a Python list. Now computed once with np.concatenate instead of list wrapping. #6 — bin_and_profile_2D: nested loop → np.meshgrid Replaced the double Python loop over bin centres with a single np.meshgrid call + .ravel(), keeping the same flat-array layout (index = y_bin_index * n_xbins + x_bin_index). Bonus — Module-level np.finfo constants np.finfo(float).max and .eps (called ~20 times across function signatures and bodies) are now computed once at import time as _FLOAT_MAX and _FLOAT_EPS.
What was changed: Correctness / safety - read_hdf5_datasets: HDF5 file handle now uses a with context manager — file is guaranteed to close even if an exception occurs during reading - 7 functions: mutable default [] arguments for confidence_levels, credible_regions, and contour_levels changed to None with a guard at the top of each function body Dead code removal - bin_and_profile_1D: removed 7 lines of commented-out old binning implementation - create_empty_figure_1D / 2D: removed unused figheight_figwidth_ratio variable - plot_1D_profile, plot_1D_delta_chi2, plot_2D_profile, plot_2D_posterior, plot_1D_posterior, plot_2D_scatter: removed unused n_pts assignments - plot_1D_profile: removed redundant double-initialisation of cl_lines_y_vals Style fixes - plot_2D_profile, nearest_neighbor_averaging: != None → is not None - plot_1D_profile, plot_1D_delta_chi2, plot_2D_profile, plot_2D_posterior, plot_2D_scatter: -> None return annotations corrected to -> tuple: - plot_conditional_profile_intervals, plot_conditional_credible_intervals: two previously-missed fontsize - 3 hardcodings fixed to use plot_settings["colorbar_tick_label_fontsize_offset"] New private helpers (5 functions added before create_empty_figure_1D) - _expand_2D_bounds(xy_bounds) — replaces 4-line epsilon-expansion block, used in 5 functions - _draw_threshold_lines(...) — replaces duplicated CL/CR line-drawing loops in plot_1D_profile and plot_1D_delta_chi2 - _setup_colorbar(...) — replaces ~15-line colorbar setup block in 5 functions - _plot_marker(...) — replaces 2–5-line ax.scatter marker calls in 8 places - _x_condition_mask(...) — replaces duplicated bin-masking if/elif blocks in both conditional-interval functions
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
1 participant
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.
These are a bunch of changes suggested by claude.