Removed tullio dependency

[simone-silvestri]

No description provided.

[glwagner]

Daaang.

[tomchor]

Does this have advantages in terms of speed?

[simone-silvestri]

I don't thinks so. I'll try to benchmark it

[glwagner]

Does this have advantages in terms of speed?

I think we hope the code is similar under the hood but it's hard to say without knowing more about Tullio.

This update is more general though, because it will work out of the box on distributed systems / multiple GPUs / multi region scenarios like the cubed sphere (because we will support reductions for those cases).

[francispoulin]

I admit that I had to look up Tullio.jl to learn that it helps with matrix operations. Good for me to know, but if it's going away then maybe no longer essential.

I gather that abs is being replaced with another version. Where does the new version come from?

Also, I remember a while ago having issues with computing norms on GPUs. Does this help with that at all?

[simone-silvestri]

abs is the classical one, I just needed to add it to the list of unary operators that can be applied as an AbstractOperation to a field. I think norm is defined for Fields but not as a "lazy" operation (i.e., it always immediately returns a value)

[glwagner]

@simone-silvestri is this right? I think the CFL constraint applies in every direction independently.

[glwagner]

Suggested change

	min_timescale = minimum(1 / (abs(u) / Δx + abs(v) / Δy + abs(w) / Δz))
	min_timescale = 1 / max(maximum(abs(u) / Δx), maximum(abs(v) / Δy), maximum(abs(w) / Δz))

[simone-silvestri]

I think in this way you don't account for diagonal motion?

[glwagner]

I guess wikipedia disagrees with me:

https://en.wikipedia.org/wiki/Courant%E2%80%93Friedrichs%E2%80%93Lewy_condition#The_two_and_general_n-dimensional_case

But I'm not sure why. I'll change the tests....

[simone-silvestri]

I think it has to do with diagonal motion...

[glwagner]

🤔

[glwagner]

This operation is doing interpolation under the hood because of the staggered grid. What's the correct CFL condition for a staggered grid?

[glwagner]

@simone-silvestri I refactored the CFL calculation so that we only have one function all the time, rather than one "ordinary" and one "accurate".

The tests still fail though. I'm not totally sure why, but I am somewhat confused how this is supposed to work on a staggered grid. I don't think we can apply wikipedia's definition; we need the definition that's correct for a C grid.

[simone-silvestri]

The difference between here and the tullio implementation is that here there is an implicit interpolation while the tullio version does not interpolate (just sums values at different locations)

[simone-silvestri]

The correct implementation should be without interpolation on a C grid. I am not entirely sure on why... i can check some references

[glwagner]

Hmm ok! Just to clarify the Tullio implementation didn't interpolate (explicitly or implicitly). Instead it assumed it was valid to add objects at different locations, I guess. I think that's what you meant though by "implicit interpolation".

[glwagner]

I guess this means that we need abstract operations to be a little less strict 😂

[glwagner]

I think we can build it manually and avoid some of the auto-interpolating stuff.

[glwagner]

Oy, I guess we're still defeated here.

[navidcy]

This smells stale… what should we do?

[glwagner]

I think the conclusion is that we need to use mapreduce to avoid Tullio, but we can't use AbstractOperations (because the CFL formula involves an operation between fields at different locations, which AbstractOperations are not designed to do)

[navidcy]

Okie. We can close then?

[glwagner]

If @simone-silvestri is ok to shelve and revisit later then yes

[simone-silvestri]

let's do it