interface for navier stokes krylov solver

[awccoppFC]

Note

Medium Risk
Touches core solver configuration, translation, and validation paths; incorrect defaults or restriction checks could change solver JSON output or block previously valid setups.

Overview
Adds a Krylov-solver configuration surface to NavierStokesSolver via use_krylov_solver, new LinearSolver Krylov fields, and a new LineSearch model (exported from flow360/__init__.py).

When enabled, model validation auto-populates solver defaults (and errors/warns on incompatible/ignored settings); the solver JSON translator injects the corresponding maxPreconditionerIterations, krylovRelativeTolerance, and lineSearch blocks and strips Krylov-only fields when disabled. SimulationParams validation now rejects Krylov usage with velocity/pressure-density limiters and with Unsteady time stepping.

^{Written by Cursor Bugbot for commit f57875d. This will update automatically on new commits. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

[cursor]

Translator omits cleanup of Krylov fields when disabled

Low Severity

When use_krylov_solver is False, the translator pops maxPreconditionerIterations from the linear solver dict but does not pop krylovRelativeTolerance or lineSearch. The model validator only logs a warning (rather than raising) when krylov_relative_tolerance or line_search are set with krylov disabled, so those values survive serialization via dump_dict and leak into the translated backend config. The cleanup intent is clear from the maxPreconditionerIterations pop, but the other two Krylov-specific fields are missed.

Additional Locations (1)

• flow360/component/simulation/models/solver_numerics.py#L210-L220

 

[Copilot]

Pull request overview

This pull request adds a Krylov solver configuration interface to the NavierStokesSolver class, enabling users to configure advanced Newton-Krylov solver options for solving the Navier-Stokes equations. The changes introduce a new LineSearch model for line search parameters, extend the LinearSolver model with Krylov-specific fields, and add validation to prevent incompatible configurations (e.g., using Krylov solver with velocity/pressure-density limiters or unsteady time stepping).

Changes:

• New LineSearch model with configurable parameters for residual growth control
• Extended LinearSolver with max_preconditioner_iterations and krylov_relative_tolerance fields
• Added use_krylov_solver flag and line_search field to NavierStokesSolver
• Validation logic to restrict Krylov solver usage with incompatible settings
• Translator logic to inject/strip Krylov-specific parameters in solver JSON output

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
flow360/init.py	Exports the new LineSearch class for public API access
flow360/component/simulation/models/solver_numerics.py	Defines LineSearch class, adds Krylov fields to LinearSolver, and implements model validator to populate Krylov defaults
flow360/component/simulation/simulation_params.py	Registers the Krylov restrictions validator in SimulationParams
flow360/component/simulation/translator/solver_translator.py	Handles conditional JSON output for Krylov parameters based on use_krylov_solver flag
flow360/component/simulation/validation/validation_simulation_params.py	Implements validation logic to prevent Krylov solver with incompatible settings

---

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

[Copilot]

There's inconsistent error handling when use_krylov_solver=False. The code raises a ValueError for max_preconditioner_iterations (line 208-210) but only logs warnings for krylov_relative_tolerance (lines 212-215) and line_search (lines 216-220). This inconsistency is confusing - either all three should raise errors (since they're all Krylov-specific), or all should be warnings. Consider making the behavior consistent across all Krylov-specific fields.

Suggested change

	log.warning(
	"krylov_relative_tolerance is set but use_krylov_solver is False. "
	"This value will be ignored."
	)
	if self.line_search is not None:
	log.warning(
	"line_search is set but use_krylov_solver is False. "
	"This value will be ignored."
	raise ValueError(
	"krylov_relative_tolerance can only be set when use_krylov_solver=True."
	)
	if self.line_search is not None:
	raise ValueError(
	"line_search can only be set when use_krylov_solver=True."

[Copilot]

When use_krylov_solver is False, the code should also remove krylovRelativeTolerance from the linearSolver dict to prevent it from appearing in the solver JSON output. Currently, only maxPreconditionerIterations is removed, but krylovRelativeTolerance is Krylov-specific and should also be stripped when the Krylov solver is disabled.

Suggested change

	ls_dict.pop("maxPreconditionerIterations", None)
	ls_dict.pop("maxPreconditionerIterations", None)
	ls_dict.pop("krylovRelativeTolerance", None)

[Copilot]

When use_krylov_solver is False, the lineSearch field should also be removed from the navierStokesSolver dict. The line_search configuration is specific to the Krylov solver and should not appear in the JSON when use_krylov_solver=False. The model validator in solver_numerics.py already warns about this (lines 216-220), so the translator should strip it out to maintain consistency.

Suggested change

	ls_dict.pop("maxPreconditionerIterations", None)
	ls_dict.pop("maxPreconditionerIterations", None)
	ns_dict.pop("lineSearch", None)

[Copilot]

The use_krylov_solver field should be excluded from the navierStokesSolver JSON output as it's a Python-side control flag, not a solver parameter. Currently, when model.navier_stokes_solver is dumped to JSON via dump_dict(), use_krylov_solver will be included as "useKrylovSolver" (camelCase). This field controls whether Krylov-specific parameters are populated but isn't itself a solver configuration parameter. Consider adding ns_dict.pop("useKrylovSolver", None) after line 2245 to remove it from the output.

Suggested change

	ns_dict = translated["navierStokesSolver"]
	ns_dict = translated["navierStokesSolver"]
	# use_krylov_solver is a Python-side control flag; do not expose it in JSON
	ns_dict.pop("useKrylovSolver", None)

[Copilot]

The validation logic iterates through the models list twice (lines 925-943 and 946-951). The second loop checks for Krylov solver usage with Unsteady time stepping, which could be combined with the first loop for better efficiency. Consider moving the time_stepping check inside the first loop after checking use_krylov_solver to avoid redundant iteration.

Suggested change

	if params.time_stepping is not None and isinstance(params.time_stepping, Unsteady):
	for model in models:
	if isinstance(model, Fluid) and model.navier_stokes_solver.use_krylov_solver:
	raise ValueError(
	"The Krylov solver (use_krylov_solver=True) is not supported with "
	"Unsteady time stepping. Please use Steady time stepping."
	)
	if params.time_stepping is not None and isinstance(params.time_stepping, Unsteady):
	raise ValueError(
	"The Krylov solver (use_krylov_solver=True) is not supported with "
	"Unsteady time stepping. Please use Steady time stepping."
	)

[Copilot]

There is duplication of default value logic between the model validator (_populate_krylov_defaults in solver_numerics.py lines 223-230) and the translator (lines 2249-2257). The model validator already sets these defaults when use_krylov_solver=True, so the translator should not need to check and set them again. This duplication could lead to inconsistencies if defaults are updated in one place but not the other. Consider removing the default-setting logic from the translator and relying solely on the model validator, or add a comment explaining why both are necessary.

Suggested change

	elif "maxPreconditionerIterations" not in ls_dict:
	ls_dict.setdefault("maxPreconditionerIterations", 25)
	ls_dict.setdefault("krylovRelativeTolerance", 0.05)
	if "lineSearch" not in ns_dict:
	ns_dict["lineSearch"] = {
	"residualGrowthThreshold": 0.85,
	"maxResidualGrowth": 1.1,
	"activationStep": 100,
	}

[Copilot]

The residual_growth_threshold field is typed as float without constraints, but based on the description and typical usage, it should likely be constrained. The description states it's a "convergence ratio above which no residual increase (RHS > 1.0) is allowed," suggesting it should be between 0 and 1. Consider using pd.confloat(ge=0, le=1) or a similar constraint to ensure valid values.

Suggested change

	residual_growth_threshold: float = pd.Field(
	residual_growth_threshold: pd.confloat(ge=0, le=1) = pd.Field(

[Copilot]

The max_residual_growth field is typed as float without constraints. Given it represents a "hard cap on RHS ratio," it should likely be constrained to positive values. Consider using PositiveFloat or pd.confloat(gt=0) to ensure the value makes sense in the context of a growth factor.

Suggested change

	max_residual_growth: float = pd.Field(
	max_residual_growth: PositiveFloat = pd.Field(