Skip to content

Commit b940b12

Browse files
CopilotocramzMarco Zocca
authored
Re-enable BiCGSTAB and linSolve0 iterative solvers (#95)
* Initial plan * Re-enable BiCGSTAB and linSolve0 with comprehensive tests Co-authored-by: ocramz <5902760+ocramz@users.noreply.github.com> * Fix linSolve0 to use IterE exception instead of undefined NotImplementedError Co-authored-by: ocramz <5902760+ocramz@users.noreply.github.com> * Update documentation for BiCGSTAB and linSolve0 re-enablement Co-authored-by: ocramz <5902760+ocramz@users.noreply.github.com> * Add build and test instructions for BiCGSTAB and linSolve0 Co-authored-by: ocramz <5902760+ocramz@users.noreply.github.com> * Address code review feedback: fix CGNE algorithm, add exports, remove dead code Co-authored-by: ocramz <5902760+ocramz@users.noreply.github.com> * Minor improvements: better error message, clarify zero vector idiom, fix doc Co-authored-by: ocramz <5902760+ocramz@users.noreply.github.com> * Fix linSolve0 type error: make solver function polymorphic over state types Co-authored-by: ocramz <5902760+ocramz@users.noreply.github.com> * Fix linSolve0 type signature: add constraints for CGNE solver Co-authored-by: ocramz <5902760+ocramz@users.noreply.github.com> * Fix linSolve0: add Monad constraint and type annotation for IterE Co-authored-by: ocramz <5902760+ocramz@users.noreply.github.com> * Fix linSolve0: remove explicit type signature from runSolver to allow proper type inference Co-authored-by: ocramz <5902760+ocramz@users.noreply.github.com> * fix bicgstab and generator of spd matrices * Update README: document BiCGSTAB re-enablement and property test behavior Co-authored-by: ocramz <5902760+ocramz@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ocramz <5902760+ocramz@users.noreply.github.com> Co-authored-by: Marco Zocca <marco.zocca@unfoldml.com>
1 parent a8ebde2 commit b940b12

File tree

7 files changed

+680
-85
lines changed

7 files changed

+680
-85
lines changed

‎.github/copilot-instructions.md‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
## Engineering and workflow conventions
2+
3+
## Workflow
4+
Use the Makefile : `make build`, `make test`, which internally use the `stack` CLI.
5+
6+
### Clarity before code
7+
Do not jump into coding new features or fixing bugs without first fully understanding the requirements and implications.
8+
Always discuss with the user or team to clarify any ambiguities before starting implementation, and ask follow up questions if needed.
9+
10+
### NO SHORTCUTS
11+
Never leave incomplete functions or stubs in the codebase. Every function must be fully implemented or explicitly marked as unimplemented with a clear error message.
12+
Never delete or comment out tests to bypass failures; instead, investigate and resolve the underlying issues.
13+
Never, ever, use "In a real implementation," or similar phrases to justify incomplete code or post-hoc defaults.
14+
Do /not/ mark functions as TODO or FIXME without prior approval from the user. Scope down a feature if needed, and tell the user what's missing.
15+
16+
### Testing and validation
17+
A feature is not complete until it has comprehensive tests that validate its correctness and robustness.
18+
Meaningful tests rather than coverage: Aim for meaningful, high-quality tests that thoroughly validate core functionality and edge cases.
19+
Do not test trivial properties of data structures, but aim for domain-specific validation.
20+
21+
22+
### Debugging and troubleshooting
23+
When investigating or debugging, do not skip or modify tests, or mark them as pending, without receiving prior approval from the user.
24+
25+
26+
### Module Imports
27+
All imports must be explicit. If a module does not export an explicit list, import only the needed symbols. When importing an external library, import only the required functions/types.
28+
29+
### Errors and exceptions
30+
* Never use `error` or `undefined`
31+
* functions with alternative return types should use sum types (e.g. Maybe, Either, or custom ones)
32+
* All error messages should be informative and include context about the failure that can be used to fix the issue.
33+

‎BUILD_AND_TEST.md‎

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,124 @@
1+
# Build and Test Instructions
2+
3+
This document provides instructions for building and testing the BiCGSTAB and linSolve0 re-enablement.
4+
5+
## Prerequisites
6+
7+
- Stack build tool installed
8+
- Internet connection for downloading dependencies
9+
10+
## Build
11+
12+
```bash
13+
# Build the project
14+
stack build
15+
16+
# Or using make
17+
make build
18+
```
19+
20+
Expected output: Successful compilation with no errors.
21+
22+
## Test
23+
24+
### Run All Tests
25+
26+
```bash
27+
# Run full test suite
28+
stack test
29+
30+
# Or using make
31+
make test
32+
```
33+
34+
### Run Specific Test Suites
35+
36+
```bash
37+
# Test BiCGSTAB only
38+
stack test --test-arguments "-m BiCGSTAB"
39+
40+
# Test CGS only
41+
stack test --test-arguments "-m CGS"
42+
43+
# Test linSolve0 interface
44+
stack test --test-arguments "-m linSolve"
45+
```
46+
47+
## Expected Test Results
48+
49+
### BiCGSTAB Tests
50+
- ✅ bicgsInit creates initial BiCGSTAB state
51+
- ✅ bicgstabStep performs one iteration
52+
- ✅ BiCGSTAB converges on 2x2 system
53+
- ✅ BiCGSTAB converges on 3x3 SPD system
54+
- ✅ prop_bicgstab : BiCGSTAB converges for SPD systems (100 QuickCheck cases)
55+
56+
### linSolve0 Tests
57+
- ✅ BiCGSTAB (2 x 2 dense)
58+
- ✅ BiCGSTAB (3 x 3 sparse, symmetric pos.def.)
59+
- ✅ CGS (2 x 2 dense)
60+
- ✅ CGS (3 x 3 sparse, SPD)
61+
- ✅ CGNE (2 x 2 dense)
62+
- ✅ CGNE (3 x 3 sparse, SPD)
63+
64+
### CGS Tests (Existing, should still pass)
65+
- ✅ cgsInit creates initial CGS state
66+
- ✅ cgsStep performs one iteration
67+
- ✅ CGS converges on 2x2 system
68+
- ✅ CGS converges on 3x3 SPD system
69+
- ✅ prop_cgs : CGS converges for SPD systems (100 QuickCheck cases)
70+
71+
## Troubleshooting
72+
73+
### If Tests Fail
74+
75+
1. **Check tolerance settings**: The tests use lenient tolerances (1e-4 relative, 1e-6 absolute)
76+
- If tests fail due to convergence issues, consider increasing the number of iterations
77+
- Location: `test/LibSpec.hs`, functions `checkBiCGSTAB`, `checkCGS`
78+
79+
2. **Check system properties**: Property tests guard against degenerate cases:
80+
- Systems smaller than 3x3
81+
- Nearly zero RHS or solution vectors
82+
- Very sparse matrices with few non-zeros
83+
- Large sparse systems (n > 20 with density < 0.1)
84+
85+
3. **Debug with trace output**: Use `checkCGSDebug` function for detailed iteration output
86+
87+
4. **Verify preconditioning**: Consider adding preconditioning for ill-conditioned systems
88+
89+
### Network Issues
90+
91+
If `stack build` fails due to network timeouts:
92+
93+
1. Try again - sometimes Stackage servers are temporarily unavailable
94+
2. Check your internet connection
95+
3. Try using a different resolver in `stack.yaml`
96+
4. Use cached dependencies if available: `stack build --no-install-ghc`
97+
98+
## Verification Checklist
99+
100+
Before considering the task complete:
101+
102+
- [ ] `stack build` completes successfully with no warnings
103+
- [ ] All BiCGSTAB tests pass
104+
- [ ] All linSolve0 tests pass
105+
- [ ] All existing tests (CGS, etc.) still pass
106+
- [ ] No new compiler warnings introduced
107+
- [ ] Documentation is up to date
108+
109+
## Performance Notes
110+
111+
- BiCGSTAB typically converges faster than CGS on most systems
112+
- BiCGSTAB is more stable for ill-conditioned matrices
113+
- linSolve0 uses 200 fixed iterations - convergence usually happens much sooner
114+
- Property tests may take a few seconds due to QuickCheck generating 100 test cases
115+
116+
## Next Steps
117+
118+
After successful build and test:
119+
120+
1. Review test output for any warnings
121+
2. Check if any tests are flaky or timeout
122+
3. Consider adding preconditioner support
123+
4. Benchmark performance against other solvers
124+
5. Add more comprehensive documentation/examples

‎COMMENTED_OUT_FUNCTIONS.md‎

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ This document lists all the functions that were commented out during the process
44

55
**Status Updates:**
66
- ✅ **CGS (Conjugate Gradient Squared)**: Re-enabled with comprehensive tests (see issue/PR for re-enabling CGS)
7+
- ✅ **BiCGSTAB (Biconjugate Gradient Stabilized)**: Re-enabled with comprehensive tests
78

89
## File: src/Numeric/LinearAlgebra/Sparse.hs
910

@@ -83,10 +84,12 @@ cgsStep aa rhat (CGS x r p u) = CGS xj1 rj1 pj1 uj1
8384

8485
---
8586

86-
### 3. BiCGSTAB (Biconjugate Gradient Stabilized) Solver Functions
87+
### 3. ✅ BiCGSTAB (Biconjugate Gradient Stabilized) Solver Functions - **RE-ENABLED**
88+
89+
**Status**: Functions have been uncommented, exported from the module, and comprehensive tests added.
8790

8891
#### `bicgsInit`
89-
**Lines**: 950-954 (commented)
92+
**Status**: ✅ Active (lines 961-964)
9093
```haskell
9194
bicgsInit :: LinearVectorSpace (SpVector a) =>
9295
MatrixType (SpVector a) -> SpVector a -> SpVector a -> BICGSTAB a
@@ -96,7 +99,7 @@ bicgsInit aa b x0 = BICGSTAB x0 r0 r0 where
9699
**Purpose**: Initializes the BiCGSTAB solver state
97100

98101
#### `bicgstabStep`
99-
**Lines**: 956-971 (commented)
102+
**Status**: ✅ Active (lines 966-977)
100103
```haskell
101104
bicgstabStep :: (V (SpVector a), Fractional (Scalar (SpVector a))) =>
102105
MatrixType (SpVector a) -> SpVector a -> BICGSTAB a -> BICGSTAB a
@@ -113,6 +116,12 @@ bicgstabStep aa r0hat (BICGSTAB x r p) = BICGSTAB xj1 rj1 pj1 where
113116
```
114117
**Purpose**: Performs one iteration step of the BiCGSTAB algorithm
115118

119+
**Tests Added**:
120+
- Unit tests for `bicgsInit` (verifies initial state)
121+
- Unit tests for `bicgstabStep` (verifies iteration works)
122+
- Integration tests on 2x2 and 3x3 systems
123+
- Property-based test for SPD (symmetric positive definite) systems
124+
116125
---
117126

118127
### 4. Moore-Penrose Pseudoinverse
@@ -145,13 +154,12 @@ class IterativeSolver s where
145154
## Summary Statistics
146155

147156
- **Total functions originally commented out**: 10
148-
- **Re-enabled**: 2 (CGS)
149-
- **Still commented out**: 8
157+
- **Re-enabled**: 4 (CGS + BiCGSTAB)
158+
- **Still commented out**: 6
150159
- BCG-related: 2 functions
151-
- BiCGSTAB-related: 2 functions
152160
- Pseudoinverse: 1 function
153161
- Typeclass: 1 class definition
154-
- Show instances: 3 instances (for BCG, CGS, BICGSTAB)
162+
- Show instances: 1 instance (for BCG)
155163

156164
## Reason for Commenting Out
157165

0 commit comments

Comments
 (0)