Fix ConjugateGradientPoissonSolver construction with default pre-conditioner

[ali-ramadhan]

I added a test that fails due to #3829. I should also test the non-FFT case and test that the proper pre-conditioner was initialized. So the test could be better but it does fail as it should.

What we need is a using Oceananigans.ImmersedBoundaries but the solvers module is defined well before the immersed boundaries module.

So for a solver to depend on the immersed boundaries module, and really just the ImmersedBoundaryGrid type then the immersed boundaries module needs to be included first.

Based on these comments maybe it's desirable to change the order of inclusion? But maybe it'll take some work. So otherwise we probably need to define another abstract type in src/Oceananigans.jl but this solution isn't ideal.

Oceananigans.jl/src/Oceananigans.jl

Lines 214 to 228 in 13bf409

	# TODO: move here
	#include("ImmersedBoundaries/ImmersedBoundaries.jl")
	#include("MultiRegion/MultiRegion.jl")
	# Physics, time-stepping, and models
	include("Coriolis/Coriolis.jl")
	include("BuoyancyModels/BuoyancyModels.jl")
	include("StokesDrifts.jl")
	include("TurbulenceClosures/TurbulenceClosures.jl")
	include("Forcings/Forcings.jl")
	include("Biogeochemistry.jl")
	# TODO: move above
	include("ImmersedBoundaries/ImmersedBoundaries.jl")
	include("Models/Models.jl")

Resolves #3829

[glwagner]

Suggested change

	construct_conjugate_gradient_poisson_solver_with_default_preconditioner(grid)
	solver = ConjugateGradientPoissonSolver(grid)
	@test solver isa ConjugateGradientPoissonSolver

[glwagner]

Suggested change

	function construct_conjugate_gradient_poisson_solver_with_default_preconditioner(grid)
	solver = ConjugateGradientPoissonSolver(grid)
	return true
	end

[glwagner]

Oof, good catch. Thanks for adding the test.

As a temporary fix, we could add a special constructor for ImmersedBoundaryGrid (ie use dispatch rather than the if-statement).

I suspect re-ordering the imports will be a larger piece of work that will probably require some work from @simone-silvestri . Technical debt is entrenched.

[simone-silvestri]

To do this, we need to shift the definitions of

• immersed tridiagonal solver (primarily in abstract_grid_fitted_boundary.jl) in the Solver or TurbulenceClosures module
• distributed immersed boundary definitions (the distributed_immersed_boundaries.jl file mostly) in the DistributedComputations module
• the viscous fluxes (in conditional_fluxes.jl) in the TurbulenceClosure module
• probably better at this point to include ImmersedBoundaries before the Advection module and shift the advective fluxes definition (in conditional_fluxes.jl) in Advection

This will make the ImmersedBoundaries module very light weight and the others a bit more complicated but it something we discussed already and I think it's a positive change now that immersed boundaries are less "experimental". @ali-ramadhan I can help on this PR if you want.

[glwagner]

It'd be easier to understand the suggestions if you put the links to the source code in your comment.

[ali-ramadhan]

Thanks for the suggestions @simone-silvestri! And I noticed you started doing some refactoring in PR #3847 so I'll close this in favor of your PR.