Skip to content

Add and expose typed PreconditionerVariant - #251

Open
leostimpfle wants to merge 1 commit into
py-econometrics:mainfrom
leostimpfle:feat/expose-preconditioner-variant
Open

Add and expose typed PreconditionerVariant#251
leostimpfle wants to merge 1 commit into
py-econometrics:mainfrom
leostimpfle:feat/expose-preconditioner-variant

Conversation

@leostimpfle

@leostimpfle leostimpfle commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

This PR addresses the "variant" part of #236. I'll address the "build times" in a second PR.

I've made three key design choices (which can be debated):

  • Introduced an exhaustive Rust PreconditionerVariant enum representing the algorithm of a built preconditioner: Additive or Diagonal.
  • This is kept separate from the non-exhaustive PreconditionerConfig: the config describes a construction request while the variant describes what an existing Preconditioner contains.
  • Kept Preconditioner::variant_name() for backward compatibility, but deprecated it in favour of the typed PreconditionerVariant. (This is mostly to avoid a having to bump the version to 0.4.0.)

@schroedk

schroedk commented Aug 12, 2026

Copy link
Copy Markdown
Collaborator

@leostimpfle I was wondering if we could just ship the config with Preconditioner, i.e.

#[derive(Clone, Serialize, Deserialize)]
pub(crate) struct FeSchwarz {
    inner: SchwarzPreconditioner<BlockElimSolver>,
    config: PreconditionerConfig,
}

impl FeSchwarz {
    fn config(&self) -> &PreconditionerConfig { &self.config }
}

#[derive(Clone, Debug, Serialize, Deserialize)]
struct DiagonalPreconditioner { inv_diag: Arc<[f64]> }

impl DiagonalPreconditioner {
    fn config(&self) -> &PreconditionerConfig {
        const DIAGONAL: PreconditionerConfig = PreconditionerConfig::Diagonal;
        &DIAGONAL
    }
}

impl Preconditioner {

    pub fn config(&self) -> &PreconditionerConfig {
        match &self.inner {
            Variant::Additive(p) => p.config(),
            Variant::Diagonal(d) => d.config(),
        }
    }
}

then we don't need a new type at all. I haven't worked out the Python side though. Also I would not keep variant_name for backwards compatibility (we have to make a breaking release anyhow).

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.

2 participants