Skip to content

refactor: simplify clenshaw summations - #175

Merged
tsutterley merged 2 commits into
mainfrom
clenshaw
Jul 13, 2026
Merged

refactor: simplify clenshaw summations#175
tsutterley merged 2 commits into
mainfrom
clenshaw

Conversation

@tsutterley

Copy link
Copy Markdown
Owner

No description provided.

@tsutterley tsutterley changed the title refactor: simply clenshaw summations refactor: simplify clenshaw summations Jul 13, 2026
@tsutterley
tsutterley requested a review from Copilot July 13, 2026 18:07

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors how the complex longitude phase terms (exp(1j*m*phi)) are generated for Clenshaw-style spherical harmonic summations, and updates CI path filters so Python tests run when additional top-level packages change.

Changes:

  • Refactors exp(1j*m*phi) construction in both sea_level_equation and clenshaw_summation to a fully-vectorized np.exp(...) computation.
  • Renames the _clenshaw return variable from s_m-style naming to cs_m for consistency with callers.
  • Expands the .github/workflows/python-request.yml pull_request path filters to include additional package directories.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
gravity_toolkit/sea_level_equation.py Precomputes m_phi via vectorized exponentials outside the iterative solver loop.
gravity_toolkit/clenshaw_summation.py Vectorizes m_phi generation and renames _clenshaw internal/return variable to cs_m.
.github/workflows/python-request.yml Triggers the Python workflow on changes in more top-level source directories.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread gravity_toolkit/clenshaw_summation.py Outdated
Comment thread gravity_toolkit/sea_level_equation.py
refactor: `ruff` format
ci: add `ruff` format check
@tsutterley

Copy link
Copy Markdown
Owner Author

@copilot review the updated gravity_toolkit/sea_level_equation.py and gravity_toolkit/clenshaw_summation.py programs

Copilot AI commented Jul 13, 2026

Copy link
Copy Markdown

@copilot review the updated gravity_toolkit/sea_level_equation.py and gravity_toolkit/clenshaw_summation.py programs

Reviewed the current committed state at d052157. The backward phase recurrence in both files is numerically consistent, the focused test/test_sea_level.py check passes, and CodeQL is clean. I only found one minor follow-up in gravity_toolkit/sea_level_equation.py: the np.multiply(cs_m[jj], m_phi_current[ii]).real expression could be simplified to direct multiplication for consistency with clenshaw_summation.py, but I did not make code changes.

@tsutterley
tsutterley merged commit 041afed into main Jul 13, 2026
8 checks passed
@tsutterley
tsutterley deleted the clenshaw branch July 13, 2026 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants