Fix some Solvers not always using user preconditioner/Jacobian

[ZedThree]

CVODE, ARKODE and PETSc Solvers only use the preconditioner or Jacobian if they were set using Solver::setPrecon/setJacobian, and not if they were set through the PhysicsModel versions.

This required adding Solver::hasUserJacobian/runJacobian in order to access the model or solver Jacobian seamlessly.

Discovered because I've started removing the old Solver API and realised the PETSc solver stored its own pointers to the preconditioner/Jacobian functions! I'm not sure if anyone actually uses these functions, or this combination of solver and model functions, so possibly this doesn't affect anyone.

Should the name be Solver::hasJacobian instead of hasUserJacobian?

[github-actions]

clang-tidy made some suggestions

[johnomotani]

Just a comment: at least for CVODE the preconditioner-setting was working (I've used it in 4.4 and it seemed to do something), and it looks like you didn't need to change the preconditioner stuff here (apart from deleting the overrides in PetscSolver, which I've never used, and hiding away the use of function pointers).

I'd vote for hasJacobian() rather than hasUserJacobian() and rename has_user_precon() to hasPrecon() while you're here. I can't imagine we'll ever be able to automatically build preconditioners (even something like algbraic multigrid with Hypre you'd have to build it a matrix somehow...) so there's no non-user preconditioner or Jacobian to distinguish.

[ZedThree]

The issue is that there is a PhysicsModel::setPrecon method that takes a physics model member function which is stored in the model, while the Solvers took a pointer to a free function that's stored in the Solver. The Solvers fixed here had overrides for Solver::setPrecon and stored the function pointer themselves, and only checked/used that.

So this was broken:

class MyModel : public PhysicsModel {
public:
  int my_preconditioner(...) {...}

  int init(bool) {
    ...
    setPrecon((preconfunc) &MyModel::my_preconditioner); // Set in PhysicsModel but not called in CVODE etc
  }
};

but this worked:

int my_preconditioner(...) {...}
class MyModel : public PhysicsModel {
public:
  int init(bool) {
    ...
    solver->setPrecon(my_preconditioner); // Set in CVODE
  }
};

[github-actions]

clang-tidy made some suggestions

[johnomotani]

Agree fixes here are needed, and PetscSolver was broken. I think CVODE managed to work (at least precon, I haven't used Jacobian or thought about it now). CVODE would call

BOUT-dev/src/solver/solver.cxx

Lines 1427 to 1435 in 1d1307a

	int Solver::run_precon(BoutReal t, BoutReal gamma, BoutReal delta) {
	if(!have_user_precon())
	return 1;
	if(model)
	return model->runPrecon(t, gamma, delta);
	return (*prefunc)(t, gamma, delta);
	}

and get into model->runPrecon()

BOUT-dev/src/physics/physicsmodel.cxx

Lines 62 to 66 in 1d1307a

	int PhysicsModel::runPrecon(BoutReal t, BoutReal gamma, BoutReal delta) {
	if(!userprecon)
	return 1;
	return (*this.*userprecon)(t, gamma, delta);
	}

which does use the userprecon function set by PhysicsModel::setPrecon()

BOUT-dev/include/bout/physicsmodel.hxx

Line 225 in 1d1307a

void setPrecon(preconfunc pset) {userprecon = pset;}

Sorry, I'm going on about this to check I'm not missing something - I think for people who were using preconditioning with CVODE, it was doing what they expected, and it was only possibly setting the Jacobian or preconditioning with other solvers that was previously broken.

[ZedThree]

Apologies, you're entirely correct. For CVODE, it was just the jacobian that was not called correctly. The PETSc time solver was the only that didn't call the preconditioner correctly.