Skip to content

Commit f165e9e

Browse files
committed
Remove FactorLabel for simpler u32 representation
1 parent 2339897 commit f165e9e

11 files changed

Lines changed: 47 additions & 109 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ and this project follows [Semantic Versioning](https://semver.org/).
1717
- A warm start that already solves the system reports `WarmStartExact` instead of `ZeroRhs`.
1818
- **BREAKING:** `ScalingConfig::max_sweeps` is now `max_iterations`, and `BuildWarning::UnscalableComponent` reports `iterations` in place of `sweeps`; the dominance certificate runs reduced CG, not relaxation sweeps.
1919
- **BREAKING:** The serialized `Preconditioner` wire format changed with the `approx-chol` 0.4 → 0.5 bump (v12 → v13), retention of the complete construction config (v13 → v14), retention of its original build duration (v14 → v15), and the new `LocalSolverConfig::ridge` field (v15 → v16); 0.3.0 bytes no longer decode.
20+
- **BREAKING:** Coefficient addresses now use caller-visible `u32` factor labels rather than internal `usize` level positions. This affects Rust `CoefficientAddress::level` and the accepted range of Python coefficient layout and unidentified-direction levels.
2021

2122
### Added
2223

README.md

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -217,9 +217,8 @@ let r2 = solver.solve(&another_y, &LsmrOptions::default())?; // reuses precondi
217217
| `SchurMode` | `Approximate(ApproxSchurConfig)` \| `Exact` |
218218
| `Preconditioner` | Opaque built handle — reuse via `Solver::new(.., precond)` (owned or `&`) |
219219
| `Effect` | `Effect::new(levels: &[u32], intercept: bool, slopes: impl IntoIterator<Item = &[f64]>) -> Result<Self, BuildError>` |
220-
| `FactorLabel` | Opaque caller-visible label; currently constructible from `u32` and readable with `try_as_u32()` |
221-
| `CoefficientAddress` | `{ channel: Channel { term, column }, level: FactorLabel }` |
222-
| `CoefficientLayout` | `index(&CoefficientAddress) -> Option<usize>`, `address(usize) -> Option<CoefficientAddress>`, `n_dofs()`, `n_terms()`, `n_levels(term)`, `n_columns(term)` |
220+
| `CoefficientAddress` | `{ channel: Channel { term, column }, level: u32 }` |
221+
| `CoefficientLayout` | `index(CoefficientAddress) -> Option<usize>`, `address(usize) -> Option<CoefficientAddress>`, `n_dofs()`, `n_terms()`, `n_levels(term)`, `n_columns(term)` |
223222

224223
### Varying slopes
225224

@@ -241,8 +240,8 @@ let terms = vec![
241240
let r = solve(terms, &y, None, None, None)?;
242241

243242
// Read firm level 3's x-slope via the layout map (column 0 = intercept, 1 = first slope):
244-
let at = CoefficientAddress { channel: Channel { term: 0, column: 1 }, level: 3.into() };
245-
println!("{}", r.x[r.layout.index(&at).unwrap()]);
243+
let at = CoefficientAddress { channel: Channel { term: 0, column: 1 }, level: 3 };
244+
println!("{}", r.x[r.layout.index(at).unwrap()]);
246245
```
247246

248247
`Solver::new` takes the same `Vec<Effect>`, so a slope design can be reused

crates/within-py/src/results.rs

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,9 @@ impl PyCoefficientLayout {
117117
fn index(&self, term: usize, level: u32, column: usize) -> PyResult<usize> {
118118
let at = CoefficientAddress {
119119
channel: Channel { term, column },
120-
level: level.into(),
120+
level: level,
121121
};
122-
self.inner.index(&at).ok_or_else(|| {
122+
self.inner.index(at).ok_or_else(|| {
123123
PyIndexError::new_err(format!(
124124
"coefficient address (term={term}, level={level}, column={column}) out of range"
125125
))
@@ -129,13 +129,7 @@ impl PyCoefficientLayout {
129129
fn address(&self, index: usize) -> PyResult<(usize, u32, usize)> {
130130
self.inner
131131
.address(index)
132-
.map(|at| {
133-
(
134-
at.channel.term,
135-
at.level.try_as_u32().expect("u32 factor label"),
136-
at.channel.column,
137-
)
138-
})
132+
.map(|at| (at.channel.term, at.level, at.channel.column))
139133
.ok_or_else(|| {
140134
PyIndexError::new_err(format!(
141135
"x index {index} out of range (n_dofs={})",
@@ -170,7 +164,7 @@ pub(crate) fn into_py_result(py: Python<'_>, result: SolveResult) -> PySolveResu
170164
.iter()
171165
.map(|u| PyUnidentifiedDirection {
172166
term: u.channel.term,
173-
level: u.level.try_as_u32().expect("u32 factor label"),
167+
level: u.level,
174168
column: u.channel.column,
175169
})
176170
.collect(),
@@ -205,7 +199,7 @@ pub(crate) fn into_py_batch_result(
205199
.iter()
206200
.map(|u| PyUnidentifiedDirection {
207201
term: u.channel.term,
208-
level: u.level.try_as_u32().expect("u32 factor label"),
202+
level: u.level,
209203
column: u.channel.column,
210204
})
211205
.collect(),

crates/within/src/domain.rs

Lines changed: 4 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -78,36 +78,6 @@ impl<T> Loading<T> {
7878
}
7979
}
8080

81-
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
82-
enum FactorLabelValue {
83-
U32(u32),
84-
}
85-
86-
/// A caller-visible factor label.
87-
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
88-
pub struct FactorLabel(FactorLabelValue);
89-
90-
impl FactorLabel {
91-
/// Return the label as `u32`, or `None` when it has another representation.
92-
pub fn try_as_u32(&self) -> Option<u32> {
93-
match self.0 {
94-
FactorLabelValue::U32(value) => Some(value),
95-
}
96-
}
97-
}
98-
99-
impl From<u32> for FactorLabel {
100-
fn from(value: u32) -> Self {
101-
Self(FactorLabelValue::U32(value))
102-
}
103-
}
104-
105-
impl From<&FactorLabel> for FactorLabel {
106-
fn from(value: &FactorLabel) -> Self {
107-
value.clone()
108-
}
109-
}
110-
11181
/// Mapping between caller-visible factor labels and numerical level positions.
11282
///
11383
/// This is an identity mapping for the existing dense `u32` API. Later
@@ -126,16 +96,16 @@ impl FactorEncoding {
12696
self.n_levels
12797
}
12898

129-
pub(crate) fn position(&self, label: &FactorLabel) -> Option<usize> {
130-
let position = label.try_as_u32()? as usize;
99+
pub(crate) fn position(&self, label: u32) -> Option<usize> {
100+
let position = label as usize;
131101
(position < self.n_levels).then_some(position)
132102
}
133103

134-
pub(crate) fn label(&self, position: usize) -> Option<FactorLabel> {
104+
pub(crate) fn label(&self, position: usize) -> Option<u32> {
135105
if position >= self.n_levels {
136106
return None;
137107
}
138-
u32::try_from(position).ok().map(FactorLabel::from)
108+
u32::try_from(position).ok()
139109
}
140110
}
141111

@@ -484,14 +454,6 @@ mod tests {
484454
));
485455
}
486456

487-
#[test]
488-
fn u32_factor_label_round_trips() {
489-
let label = FactorLabel::from(u32::MAX);
490-
491-
assert_eq!(label.try_as_u32(), Some(u32::MAX));
492-
assert_eq!(FactorLabel::from(&label), label);
493-
}
494-
495457
#[test]
496458
fn validate_weights_checks_count_and_finiteness() {
497459
let design = Design::from_frame(frame(vec![vec![0, 0, 0, 0, 0]], vec![])).unwrap();

crates/within/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ pub use config::{
4242
ApproxCholConfig, ApproxSchurConfig, LocalSolverConfig, LsmrOptions, PreconditionerConfig,
4343
ReductionStrategy, ScalingConfig, ScalingFailure, SchurMode,
4444
};
45-
pub use domain::{Design, Effect, FactorLabel};
45+
pub use domain::{Design, Effect};
4646
pub use error::{BuildError, BuildWarning, SolveError, WithinError};
4747
pub use operator::schwarz::Preconditioner;
4848
pub use solver::{

crates/within/src/solver.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use crate::observation::ObservationFrame;
1818
use crate::operator::design::gather_apply;
1919
use crate::operator::schwarz::{build_preconditioner, Preconditioner};
2020
use crate::operator::DesignOperator;
21-
use crate::{BuildError, BuildWarning, FactorLabel, SolveError, WithinError};
21+
use crate::{BuildError, BuildWarning, SolveError, WithinError};
2222

2323
mod reparam;
2424
#[cfg(test)]
@@ -112,21 +112,21 @@ impl From<&Preconditioner> for PreconditionerInput {
112112
}
113113

114114
/// One coefficient of the design: a [`Channel`] at one level of its term.
115-
#[derive(Debug, Clone, PartialEq, Eq)]
116-
pub struct CoefficientAddress {
115+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
116+
pub struct Coefficient<L> {
117117
/// The coefficient column this address sits in.
118118
pub channel: Channel,
119-
/// Caller-visible factor label.
120-
pub level: FactorLabel,
119+
/// The level, in whichever space `L` names.
120+
pub level: L,
121121
}
122122

123-
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
124-
struct CoefficientPosition {
125-
channel: Channel,
126-
level: usize,
127-
}
123+
/// A coefficient addressed by the caller's own factor label.
124+
pub type CoefficientAddress = Coefficient<u32>;
125+
126+
/// A coefficient addressed by its internal compact position.
127+
type CoefficientPosition = Coefficient<usize>;
128128

129-
impl CoefficientPosition {
129+
impl Coefficient<usize> {
130130
fn to_caller_address(self, design: &Design) -> CoefficientAddress {
131131
let level = design.terms[self.channel.term]
132132
.encoding
@@ -196,14 +196,14 @@ impl CoefficientLayout {
196196

197197
/// Flat [`SolveResult::x`] index of `at`, or `None` if its term,
198198
/// column, or caller-visible level label is out of range.
199-
pub fn index(&self, at: &CoefficientAddress) -> Option<usize> {
199+
pub fn index(&self, at: CoefficientAddress) -> Option<usize> {
200200
let term = self.terms.get(at.channel.term)?;
201201

202202
if at.channel.column >= term.n_columns {
203203
return None;
204204
}
205205

206-
let position = term.encoding.position(&at.level)?;
206+
let position = term.encoding.position(at.level)?;
207207

208208
Some(term.offset + at.channel.column * term.encoding.n_levels() + position)
209209
}

crates/within/src/solver/tests.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use crate::Effect;
1313
fn at(term: usize, level: u32, column: usize) -> CoefficientAddress {
1414
CoefficientAddress {
1515
channel: Channel { term, column },
16-
level: level.into(),
16+
level,
1717
}
1818
}
1919

@@ -75,19 +75,19 @@ fn coefficient_layout_translates_addresses_both_ways() {
7575
assert_eq!(layout.n_levels(2), None);
7676

7777
// Forward matches the documented `offset + column * n_levels + level`.
78-
assert_eq!(layout.index(&at(0, 2, 0)), Some(2));
79-
assert_eq!(layout.index(&at(1, 0, 0)), Some(3)); // term-1 intercept, level 0
80-
assert_eq!(layout.index(&at(1, 1, 1)), Some(6)); // term-1 slope, level 1
78+
assert_eq!(layout.index(at(0, 2, 0)), Some(2));
79+
assert_eq!(layout.index(at(1, 0, 0)), Some(3)); // term-1 intercept, level 0
80+
assert_eq!(layout.index(at(1, 1, 1)), Some(6)); // term-1 slope, level 1
8181
assert_eq!(layout.n_dofs(), 7);
8282

8383
// Out-of-range coordinates are rejected, not silently wrapped.
84-
assert_eq!(layout.index(&at(1, 2, 0)), None); // level past n_levels
85-
assert_eq!(layout.index(&at(1, 0, 2)), None); // column past n_columns
86-
assert_eq!(layout.index(&at(2, 0, 0)), None); // term past n_terms
84+
assert_eq!(layout.index(at(1, 2, 0)), None); // level past n_levels
85+
assert_eq!(layout.index(at(1, 0, 2)), None); // column past n_columns
86+
assert_eq!(layout.index(at(2, 0, 0)), None); // term past n_terms
8787
assert_eq!(layout.address(7), None);
8888

8989
// `address` inverts `index` for every flat slot.
9090
for i in 0..layout.n_dofs() {
91-
assert_eq!(layout.index(&layout.address(i).expect("in range")), Some(i));
91+
assert_eq!(layout.index(layout.address(i).expect("in range")), Some(i));
9292
}
9393
}

crates/within/tests/metamorphic.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use strategies::{additive_precond, random_fe_problem_strategy};
99
fn at(term: usize, level: u32, column: usize) -> CoefficientAddress {
1010
CoefficientAddress {
1111
channel: Channel { term, column },
12-
level: level.into(),
12+
level,
1313
}
1414
}
1515

@@ -140,7 +140,7 @@ proptest! {
140140
prop_assert!(result.converged);
141141

142142
for u in &result.unidentified {
143-
let slot = result.layout.index(u).unwrap();
143+
let slot = result.layout.index(*u).unwrap();
144144
prop_assert_eq!(
145145
result.x[slot],
146146
0.0,
@@ -172,7 +172,7 @@ fn saturated_single_factor_recovers_level_means() {
172172

173173
for (level, &mean) in [2.0, 4.0, 5.0].iter().enumerate() {
174174
let label = u32::try_from(level).expect("fixture level fits u32");
175-
let slot = result.layout.index(&at(0, label, 0)).unwrap();
175+
let slot = result.layout.index(at(0, label, 0)).unwrap();
176176
assert!(
177177
(result.x[slot] - mean).abs() < 1e-6,
178178
"level {level}: coefficient {} != level mean {mean}",

crates/within/tests/properties.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use strategies::{additive_precond, random_fe_problem_strategy, random_slopes_pro
1111
fn at(term: usize, level: u32, column: usize) -> CoefficientAddress {
1212
CoefficientAddress {
1313
channel: Channel { term, column },
14-
level: level.into(),
14+
level,
1515
}
1616
}
1717

@@ -136,11 +136,11 @@ proptest! {
136136
for i in 0..n_obs {
137137
let level = f.levels[i];
138138
if f.intercept {
139-
fitted[i] += x[layout.index(&at(t, level, 0)).unwrap()];
139+
fitted[i] += x[layout.index(at(t, level, 0)).unwrap()];
140140
}
141141
for (s, col) in f.slopes.iter().enumerate() {
142142
fitted[i] +=
143-
x[layout.index(&at(t, level, slope_base + s)).unwrap()] * col[i];
143+
x[layout.index(at(t, level, slope_base + s)).unwrap()] * col[i];
144144
}
145145
}
146146
}
@@ -155,12 +155,12 @@ proptest! {
155155
let wr = weights[i] * (y[i] - fitted[i]);
156156
let wy = weights[i] * y[i];
157157
if f.intercept {
158-
let k = layout.index(&at(t, level, 0)).unwrap();
158+
let k = layout.index(at(t, level, 0)).unwrap();
159159
g[k] += wr;
160160
g0[k] += wy;
161161
}
162162
for (s, col) in f.slopes.iter().enumerate() {
163-
let k = layout.index(&at(t, level, slope_base + s)).unwrap();
163+
let k = layout.index(at(t, level, slope_base + s)).unwrap();
164164
g[k] += wr * col[i];
165165
g0[k] += wy * col[i];
166166
}

crates/within/tests/slopes.rs

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,7 @@ fn solve_single(
8989
fn drops(r: &SolveResult) -> Vec<(usize, u32, usize)> {
9090
r.unidentified
9191
.iter()
92-
.map(|d| {
93-
(
94-
d.channel.term,
95-
d.level.try_as_u32().expect("u32 factor label"),
96-
d.channel.column,
97-
)
98-
})
92+
.map(|d| (d.channel.term, d.level, d.channel.column))
9993
.collect()
10094
}
10195

@@ -264,13 +258,7 @@ fn batch_solve_shares_unidentified_and_back_transforms_each_rhs() {
264258
let batch_drops: Vec<_> = batch
265259
.unidentified
266260
.iter()
267-
.map(|d| {
268-
(
269-
d.channel.term,
270-
d.level.try_as_u32().expect("u32 factor label"),
271-
d.channel.column,
272-
)
273-
})
261+
.map(|d| (d.channel.term, d.level, d.channel.column))
274262
.collect();
275263
assert_eq!(batch_drops, [(0, 1, 1)]);
276264
// Each RHS block is bit-identical to its single solve, back-transform included.

0 commit comments

Comments
 (0)