Skip to content

Conversation

@erikvansebille
Copy link
Member

This PR fixes the way that the conversion is done for CROCO fieldsets from particle.depth (in meters) to local sigma-coordinate. Where this was initially assumed to be a simple linear conversion, this turned out to be much more complex.

After some discussion on the myroms forum https://www.myroms.org/forum/viewtopic.php?p=25752#p25752, the best option is to calculate the local depth of each sigma layer and then linearly interpolate the particle depth to find the fractional sigma depth.

This fixes #1763

@erikvansebille erikvansebille marked this pull request as ready for review December 4, 2024 07:07
@VeckoTheGecko
Copy link
Contributor

VeckoTheGecko commented Dec 4, 2024

Looks good! Just some things that I think would be good to clarify before merging.

I'm not 100% happy with the API changes with from_croco. I feel that having the hc= parameter is a deviation from the rest of the from_* functions. Though I don't have a better suggestion on how we could implement it given our limitations 🤔

@erikvansebille
Copy link
Member Author

I'm not 100% happy with the API changes with from_croco. I feel that having the hc= parameter is a deviation from the rest of the from_* functions. Though I don't have a better suggestion on how we could implement it given our limitations 🤔

Yes, I fully understand; I'm not 100% happy either.

This would actually be another reason to move to xarray for FieldSet creation in Parcels... The xarray DataSet can be expected to contain all these variables (hc, Cs_w, Zeta, H, etc) if the data comes from CROCO/Roms, so we can take them directly from there...

@VeckoTheGecko
Copy link
Contributor

This would actually be another reason to move to xarray for FieldSet creation in Parcels...

I 100% agree. I'm currently scoping that out

@erikvansebille erikvansebille merged commit 92548e1 into master Dec 5, 2024
16 checks passed
@erikvansebille erikvansebille deleted the CROCO_fix_sigma_calculation branch December 5, 2024 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

from_croco() enhancement; correct conversion from z to sigma grid

3 participants