Add space with multiple columns#2525
Conversation
7567810 to
e78f8a4
Compare
nefrathenrici
left a comment
nefrathenrici
left a comment
There was a problem hiding this comment.
This looks good, I only saw a few immediate correctness issues.
| s_lat = FT(radius) * deg2rad | ||
| s_lon = FT(radius) * deg2rad * cosd(pt.lat) | ||
| J = s_lat * s_lon # det of diagonal Jacobian | ||
| ∂x∂ξ_mat = SMatrix{2, 2, FT, 4}(s_lat, zero(FT), zero(FT), s_lon) |
There was a problem hiding this comment.
| ∂x∂ξ_mat = SMatrix{2, 2, FT, 4}(s_lat, zero(FT), zero(FT), s_lon) | |
| ∂x∂ξ_mat = SMatrix{2, 2, FT, 4}(0, s_lat, s_lon, 0) |
The axes need to be flipped. SMatrix enforces float type so I 0 is fine here.
There was a problem hiding this comment.
I tested this and it wasn't giving the same answer when comparing it against the ClimaLand single column run.
There was a problem hiding this comment.
I tested this again and they are identical (up to some tolerance). I am not sure why I was getting different answers before.
e78f8a4 to
02c2eb2
Compare
dennisYatunin
left a comment
dennisYatunin
left a comment
There was a problem hiding this comment.
This looks great! It has a fair bit of code duplication, both in terms of method definitions and const type declarations, but that's a higher-level design issue that can be fixed separately. I left a few nitpicks about readability and error handling, but overall it looks ready to merge in.
7692c10 to
bf2127c
Compare
6431488 to
cd184bd
Compare
4 participants
This PR adds a space that can supports multiple columns. This is needed to complete CliMA/ClimaLand.jl#1723.
This PR have been tested with the
FluxnetSimulations in ClimaLand and it produces identical results with the current single column used for theFluxnetSimulation.The remapping feature will not be done in this PR. For the purpose of calibration, this is not needed because the data is extracted from the ClimaCore fields rather than from the NetCDF files produced from the diagnostics.
Note that most of the code in this PR have been written with Claude.