Fes2014 Implementation#7
Conversation
Baran-Oz
commented
Aug 26, 2025
Baran-Oz
commented
- New method for tide prediction has been implemented
- Tidal constituents for both water level and tidal currents from FES2014/AVISO (https://www.aviso.altimetry.fr/en/data/products/auxiliary-products/global-tide-fes.html) are now read and used for the prediction.
- New tests are created
- Validation of the tidal water level are completed. Tidal currents validation, however, needs more focus. AVISO provides amplitude and phase for each constituent but they must be converted to the elliptic parameters, namely, major-minor axes, inclination, and amplitude.
|
|
||
| ut_constants = _add_constituent("MSQM", freq_rad_per_sec) | ||
|
|
||
| FES2014_CONSTITUENTS = [ |
There was a problem hiding this comment.
Seems like a list of files rather than a list of constituents. I suggest skipping .nc in each line or rename the variable to something that indicate that it is a list of files.
| lon, lat = _convert_FES2014_coords(lon, lat) | ||
| for cons in FES2014_CONSTITUENTS: | ||
| file_path = self.file_path / cons | ||
| name = cons.split(".")[0].upper() |
There was a problem hiding this comment.
Related to above comment. I would do rather have a list of constituents.
| name = cons.split(".")[0].upper() | ||
|
|
||
| return constituents | ||
| if name == "LA2": # Special case for LA2 |
There was a problem hiding this comment.
Why is this necessary, isn't it possible to fix this by modifying the constituent file?
There was a problem hiding this comment.
Actually, it's not super necessary and we can proceed as you want. I just thought that it would be handier for the users to just store the files as they are provided and we take care of renaming within the script. Should I remove it?
| df = ds.sel(lon=lon, lat=lat, method="nearest").to_dataframe() | ||
|
|
||
| # Check if the file is a NetCDF file or a directory | ||
| if ".nc" not in self.file_path.suffixes: |
There was a problem hiding this comment.
Make this if/else more clearer that it is either FES2014 or DTU10, checking for ".nc" is not obvious.
| ) | ||
| constituents[name] = constituent | ||
| # Check if the file is a NetCDF file or a directory | ||
| if ".nc" not in self.file_path.suffixes: |
There was a problem hiding this comment.
See comments above. Consider creating separate classes for reading FES2014 and DTU10. Then you would only need a branch in single place.
There was a problem hiding this comment.
Totally agree! I changed it
| tuple[float, float] | ||
| Latitude and Longitude in corrected format for FES2014 | ||
| """ | ||
| if lat < -90 or lat > 90: |
There was a problem hiding this comment.
Is this check necessary here? Validating user input should not be done in a conversion function.
| Amplitudes of northward (v) component | ||
| PHIv : array_like | ||
| Phase lags of v component (degrees) | ||
| plot_demo : tuple of indices (optional) |
There was a problem hiding this comment.
plot_demo is not a parameter of this function.
| return lon, lat | ||
|
|
||
|
|
||
| def ap2ep( |
There was a problem hiding this comment.
Reconsider name of function and parameters.
2 participants
