Why is `conformal_mapping` the last property of `OrthogonalSphericalShellGrid`?

[glwagner]

It seems it was put here simply for "historical reasons", ie it is the most recent parameter to be added. But that is not a good way to motivate design:

https://github.com/CliMA/Oceananigans.jl/blob/9c6ed92edf212daaa4c709c4441ce770cdd0bae3/src/Grids/orthogonal_spherical_shell_grid.jl#L46

I think it belongs second, after architecture.

Also, the type parameter should be 5th, after TZ:

https://github.com/CliMA/Oceananigans.jl/blob/9c6ed92edf212daaa4c709c4441ce770cdd0bae3/src/Grids/orthogonal_spherical_shell_grid.jl#L12

This will help with silly constructs like

https://github.com/CliMA/OrthogonalSphericalShellGrids.jl/blob/379d0de985a0927c026e6c3e43b2ebd163e054aa/src/tripolar_grid.jl#L13

cc @simone-silvestri @siddharthabishnu

[navidcy]

Indeed it was added there when I was thinking that this type was going to be used for cubed sphere only. We should remove it or think about it.

[navidcy]

The idea was that parameter would hold the required parameters to reproduce a panel of the conformal cubed sphere, that is, the dimensions in ξ-η space and the rotation of the panel with respect to the North Pole; see https://github.com/CliMA/Oceananigans.jl/blob/9c6ed92edf212daaa4c709c4441ce770cdd0bae3/src/Grids/orthogonal_spherical_shell_grid.jl#L606

[glwagner]

Ah, the property is completely essential! I'm just making a cosmetic statement about how the struct is implemented / written.

[simone-silvestri]

Agreed with both, useful to distinguish between different grids and we should put it in front.

[navidcy]

I think I was following the parameter order also of Rectilinear and LatitudeLongitude

But true, I see how it would make it very convenient to change the order, e.g., to

struct OrthogonalSphericalShellGrid{Arch, C, FT, TX, TY, TZ, A, R, FR, C} <: AbstractHorizontallyCurvilinearGrid{FT, TX, TY, TZ, Arch}

I can do that.

[simone-silvestri]

I think Arch at the end is quite convenient. We never dispatch on Arch. Also TX, TY, TZ are quite useful in front. Most of the dispatch is done on the topology. I am not sure about FT in front. FT is never used for dispatch. The most convenient ordering of parameters in my opinion would be

struct OrthogonalSphericalShellGrid{TX, TY, TZ, C..., Arch, FT}

I think also the other grids would benefit from this ordering.

[simone-silvestri]

For OrthogonalSphericalShellGrid specifically we could do

struct OrthogonalSphericalShellGrid{C, TX, TY, TZ..., Arch, FT}

since then C is removed when specializing the different grids, for example:

const CubedSpherePanelGrid{TX, TY, TZ...} = OrthogonalSphericalShellGrid{<:CubedSphereMapping, TX, TY, TZ...} where {TX, TY, TZ...}

[glwagner]

Umm. We do dispatch on arch a lot

https://github.com/CliMA/Oceananigans.jl/blob/13bf409616af8c155b72d8869b7b8f97ae0e844b/src/DistributedComputations/distributed_grids.jl#L14-L21

not sure what is "convenient" about putting it at the end.

[glwagner]

struct OrthogonalSphericalShellGrid{TX, TY, TZ, C..., Arch, FT}

I think also the other grids would benefit from this ordering.

"benefit"?

Look at the silly definitions of distributed grid above. If the number of properties change (especially if it increases) the code breaks.

We use the convention that the most frequently used type parameters come first. This is especially important in early stages of type design, so that changing the number of type parameters doesn't break code that requires knowledge of type parameters.

I'm at a total loss to comprehend why @simone-silvestri you recommend the exact opposite convention. Maybe you should make a more detailed argument.

[simone-silvestri]

Oh shoot I have completely forgot about the distributed grids