Skip to content

Commit 504fd13

Browse files
authored
Core refactoring #1: knot search, named constants, Givens rotations (#34)
2 parents 8f911a6 + 4d7af17 commit 504fd13

6 files changed

Lines changed: 1212 additions & 183 deletions

File tree

.claude/settings.local.json

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"permissions": {
3+
"allow": [
4+
"Bash(git branch:*)",
5+
"Bash(git mv:*)"
6+
],
7+
"deny": [],
8+
"ask": []
9+
}
10+
}
Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
# Fitpack Core Refactoring Summary
2+
3+
> **Style Guide**: Follow coding conventions in [CLAUDE.md](../../CLAUDE.md)
4+
5+
## Overview
6+
7+
The `src/fitpack_core.F90` file (~19,000 lines) was converted from legacy F77 code and contains
8+
repeated patterns that can be extracted into reusable routines. The full mathematical context is
9+
now available from the transcribed book (*Curve and Surface Fitting with Splines*, P. Dierckx, 1993)
10+
in `docs/book/` and papers in `docs/papers/`.
11+
12+
**Important constraints**:
13+
- Public API/interfaces cannot change
14+
- Each refactoring step = one PR, fully tested against previous version
15+
- Keep the original routine structure; no merging/consolidation of routines
16+
17+
---
18+
19+
## Completed Work
20+
21+
### PR 1: Knot Interval Search (DONE)
22+
23+
**Occurrences**: ~20 locations replaced
24+
**New routine**: `fp_knot_interval` (hybrid linear/binary search)
25+
26+
### PR 2: Named Constants (DONE)
27+
28+
**Occurrences**: ~10 replacements
29+
**Changes**: Magic numbers → `MAX_ORDER`, `DEGREE_3`, `DEGREE_5`, etc.
30+
31+
### PR 3: QR Row Rotation — Step 1 (DONE)
32+
33+
**Changes**: Added `fp_rotate_row` subroutine; replaced 1 occurrence in `fpcurf`
34+
35+
---
36+
37+
## Remaining Plan
38+
39+
Each item below is a separate PR. PRs are ordered by dependency; documentation (PR 8) can
40+
proceed in parallel with any code PR.
41+
42+
### PR 4: Complete Variant A Givens Rotations
43+
44+
**Scope**: Replace all scalar-RHS rotation patterns with `fp_rotate_row`
45+
**Occurrences**: ~15 locations in `fpcurf`, `fpcons`, `fppara`, `fprank`
46+
**Difficulty**: Low — mechanical replacement, same signature as existing helper
47+
**Variant pattern**:
48+
```fortran
49+
do i=1,k1
50+
j = j+1
51+
piv = h(i); if (equal(piv,zero)) cycle
52+
call fpgivs(piv,a(j,1),cos,sin)
53+
call fprota(cos,sin,yi,z(j))
54+
if (i<k1) call fprota(cos,sin,h(i+1:k1),a(j,2:k1-i+1))
55+
end do
56+
```
57+
**See**: [01_qr_row_rotation_refactor.md](01_qr_row_rotation_refactor.md), Variant A
58+
59+
---
60+
61+
### PR 5: Reshape Array Indexing in `fpcurf`
62+
63+
**Scope**: Clean up 1D array stride patterns in the main curve fitting routine
64+
**Goal**: Make band matrix access and multi-dimensional RHS indexing explicit,
65+
preparing the ground for cleaner Variant B/C/D rotation replacements
66+
**Difficulty**: Medium — requires careful index verification
67+
**Key patterns to address**:
68+
- Strided RHS access: `z(j:j+(idim-1)*n:n)` → clearer views
69+
- Any remaining manual offset arithmetic for band matrix columns
70+
71+
**Book references**: Ch. 4 Fig. 4.2 (band structure of E and R₁),
72+
Ch. 5 Fig. 5.1 (band structure of P and R₁*)
73+
74+
---
75+
76+
### PR 6: Variant B/C Givens Rotations
77+
78+
**Scope**: Vector RHS and two-matrix periodic variants
79+
**New routines**: `fp_rotate_row_vec` (vector RHS with stride),
80+
`fp_rotate_row_2mat` (two-matrix rotation for periodic splines)
81+
**Occurrences**: ~15 locations in `fpclos`, `fpcons`, `fppara`, `fpperi`
82+
**Difficulty**: Medium — strided access and secondary matrix add complexity
83+
**See**: [01_qr_row_rotation_refactor.md](01_qr_row_rotation_refactor.md), Variants B & C
84+
85+
---
86+
87+
### PR 7: Variant D Givens Rotations
88+
89+
**Scope**: Grid/surface fitting rotation patterns
90+
**New routine**: `fp_rotate_row_grid` (2D RHS)
91+
**Occurrences**: ~15 locations in `fpgrdi`, `fpgrre`, `fpgrsp`, `fptrnp`, `fptrpe`,
92+
`fpsurf`, `fpsphe`, `fppola`
93+
**Difficulty**: Medium — 2D indexing and varying loop structures
94+
**See**: [01_qr_row_rotation_refactor.md](01_qr_row_rotation_refactor.md), Variant D
95+
96+
---
97+
98+
### PR 8: Back-Substitution Interface
99+
100+
**Scope**: Unify `fpback` and `fpbacp` under a generic `fp_backsolve` interface
101+
**Occurrences**: ~25 call sites
102+
**Difficulty**: Low — clean abstraction, no logic changes
103+
**Book references**: Ch. 4 Eq. 4.14 (R₁·c = z₁), Ch. 5 Eq. 5.15 (R₁*·c = z₁*)
104+
105+
---
106+
107+
### PR 9+: Doxygen + MathJax Documentation
108+
109+
**Scope**: Add structured documentation headers to all core routines in `fitpack_core.F90`
110+
**Format**: See [03_doxygen_convention.md](03_doxygen_convention.md)
111+
**Content sources**: Book chapters 1-13, papers in `docs/papers/`
112+
**Approach**: Incremental — document routines as they are touched by code PRs,
113+
plus dedicated documentation-only PRs for untouched routines
114+
**Key routines** (with book references):
115+
116+
| Routine | Book Section | Key Equations | Description |
117+
|------------|----------------------------|-------------------|------------------------------------------|
118+
| `fpgivs` | §4.1.2, pp. 55-58 | 4.15, 4.16 | Givens rotation parameters |
119+
| `fprota` | §4.1.2, pp. 55-58 | 4.15 | Apply Givens rotation |
120+
| `fpback` | §4.1.2, pp. 55-58 | 4.14 | Back-substitution, bandwidth k+1 |
121+
| `fpbacp` | §6.1, pp. 95-100 | 6.6 | Back-substitution, periodic (a1+a2) |
122+
| `fpbspl` | §1.3, pp. 8-11 | 1.22, 1.30 | B-spline evaluation (de Boor-Cox) |
123+
| `fpcurf` | §4.1-5.3, pp. 53-94 | 4.12, 5.10, 5.37 | Core curve fitting algorithm |
124+
| `fpdisc` | §5.2.2, pp. 76-79 | 5.5, 5.6 | Discontinuity jumps of k-th derivative |
125+
| `fpknot` | §5.3, pp. 87-94 | 5.37-5.43 | Adaptive knot placement |
126+
| `fprati` | §5.2.4, pp. 83-86 | 5.30-5.32 | Rational interpolation for F(p) = S |
127+
| `fprank` | §9.1.2, pp. 150-152 | 9.8-9.10 | Rank-deficient system solution |
128+
| `fpchec` | §4.1.1, pp. 53-55 | 4.5, 4.17 | Schoenberg-Whitney condition check |
129+
| `fpgrdi` | §10.2, pp. 170-172 | 10.4-10.8 | Grid fitting (Kronecker decomposition) |
130+
| `fpgrre` | §10.2, pp. 170-172 | 10.4-10.8 | Grid fitting (rectangular) |
131+
| `fptrnp` | §10.2, pp. 170-172 | 10.8 | Triangularize observation matrix |
132+
| `fpsurf` | §9.1-9.2, pp. 147-167 | 9.2-9.6 | Surface fitting to scattered data |
133+
| `fpsphe` | §11.2, pp. 205-213 | 11.12-11.16 | Spherical coordinate fitting |
134+
| `fppola` | §11.1, pp. 197-205 | 11.1-11.9 | Polar coordinate fitting |
135+
136+
---
137+
138+
## Testing Strategy
139+
140+
All PRs must:
141+
1. Pass the full test suite (`fpm test`) — 49 tests, 0 failures
142+
2. Maintain `pure` procedure attributes where present
143+
3. Preserve numerical results to machine precision
144+
4. Not change any public API signatures
145+
146+
---
147+
148+
## Workflow
149+
150+
1. Each PR branches from `main` (or the previous PR's merge)
151+
2. Run `fpm test` before and after each change
152+
3. Small, focused commits within each PR
153+
4. Squash-merge to `main` for clean history

0 commit comments

Comments
 (0)