generalizing calculate_tendencies! ?

[francispoulin]

I have looked at calculate_tendencies! and had an idea and wanted some feedbakc, when you have time.

In ShallowWater for example, we define a kernel for each field, as is done in line 67 and then define the events, as is done in 74. This works well but it seems to me that when developing a new model this needs to be changed for each of the fields, which is doable but maybe more work than needed? The actual calculations are done in another function, here solution_and_trancers.jl and that has the heart of the model, so to speak.

Inspired by what we do in runge_kutta_3.jl, it seems that we could basically loop over all the fields, and also do tracers. I don't think it's going to affect the efficiency as it's going to create the same number of kernels, but it might make the code more readable, and easier to modify. We might be able to have the same calculate_tendencies! for each model and just change the other script that specifies the tendencies.

[glwagner]

For IncompressibleModel, calculate_tendencies! unpacks the fields of the model:

Oceananigans.jl/src/Models/IncompressibleModels/calculate_tendencies.jl

Lines 24 to 38 in fcd1d1b

	calculate_interior_tendency_contributions!(model.timestepper.Gⁿ,
	model.architecture,
	model.grid,
	model.advection,
	model.coriolis,
	model.buoyancy,
	model.surface_waves,
	model.closure,
	model.background_fields,
	model.velocities,
	model.tracers,
	model.pressures.pHY′,
	model.diffusivities,
	model.forcing,
	model.clock)

and thus depends on things like surface_waves, the hydrostatic pressure anomaly, and buoyancy, for example, which are not properties of all models.

Are you proposing that we unpack these fields in calculate_interior_tendency_contributions! instead?

[francispoulin]

Good question @glwagner and I am not sure of all the details but will say this.

My first thought is that for any model, we can pass surface_waves, buoyancy and whatever else we like. For shallow water, these things would not be defined and so there would be nothin to pass, I think, and therefore this would not slow things down. Do you think that's true or is there a slowdown factor that I don't appreciate? There is a lot of the inner workings of Julia that I don't understand here.

[glwagner]

It's just that if model::ShallowWaterModel then model.surface_waves will throw an error. (Computational efficiency isn't a problem here I don't think.)

[francispoulin]

Right, I see what you mean. We would need to define surface_waves, and everything else, in each model. That means that we would need model.velocities and everything else to be the same. Okay, I can see that is not desirable since we want each model to contain things that are releant to that model.

I will think about this more but see that we probably don't want the same calculate_tendencies! for each model, but they could be made a lot more similar. I might play around with this on shallow water and get back to you.

[glwagner]

I'm closing this issue because I'm judging that it's not of current, timely relevance to Oceananigans development. If you would like to make it a higher priority or if you think the issue was closed in error please feel free to re-open.