Feature/b2i marine and testing#50
Feature/b2i marine and testing#50givelberg wants to merge 6 commits intofeature/marine_obs_builderfrom
Conversation
givelberg
commented
Jun 11, 2025
givelberg
commented
- basic testing framework for spoc
- specific testing for marine b2i converters
- b2i converters
ilianagenkova
left a comment
ilianagenkova
left a comment
There was a problem hiding this comment.
I am approving this PR despite my limited understanding of SPOC. I have not tested the code.
There was a problem hiding this comment.
It looks like the satwnd processing is more unified now, and the "obstype" drives the show? (therefore lots of code was deleted). Am I correct?
|
Changed the base the the feature/marine_obs_builder branch. Once we merge into there we can merge into feature/obs_builder. |
rmclaren
left a comment
rmclaren
left a comment
There was a problem hiding this comment.
I haven't checked everything yet. Still need to review the test framework and related files.
| mask &= (lat >= -90) & (lat <= 90) & ~np.isnan(lat) | ||
|
|
||
| # Validate longitude: all in [-180, 180] or all in [0, 360] | ||
| valid_lons = lon[mask] # Longitudes where lat is valid | ||
| if len(valid_lons) == 0: | ||
| return mask | ||
|
|
||
| # Check longitude range consistency | ||
| in_neg180_180 = (valid_lons >= -180) & (valid_lons <= 180) | ||
| in_0_360 = (valid_lons >= 0) & (valid_lons <= 360) | ||
|
|
||
| # Determine if all valid longitudes are in one range | ||
| all_neg180_180 = np.all(in_neg180_180) | ||
| all_0_360 = np.all(in_0_360) | ||
|
|
||
| # If neither range is fully consistent, use dominant range | ||
| # if the fraction of the "other" range is too large, | ||
| # probably needs to throw an error | ||
| if not (all_neg180_180 or all_0_360): | ||
| if np.sum(in_neg180_180) >= np.sum(in_0_360): | ||
| mask &= (lon >= -180) & (lon <= 180) | ||
| else: | ||
| mask &= (lon >= 0) & (lon <= 360) | ||
| else: | ||
| if all_neg180_180: | ||
| mask &= (lon >= -180) & (lon <= 180) | ||
| else: | ||
| mask &= (lon >= 0) & (lon <= 360) |
There was a problem hiding this comment.
In your YAML file (mapping file), use transforms wrap to [-180, 180]. This will make this code uneccessary.
There was a problem hiding this comment.
Are you getting values outside of the proper ranges (excluding missing values)?
| # Handle undefined or None inputs | ||
| if lat is None or lon is None: | ||
| return np.array([], dtype=bool) | ||
|
|
||
| # Convert inputs to NumPy arrays, preserving masked arrays | ||
| try: | ||
| lat = np.asarray(lat, dtype=float) | ||
| lon = np.asarray(lon, dtype=float) | ||
| except (ValueError, TypeError): | ||
| return np.zeros_like(lat, dtype=bool) | ||
|
|
||
| # Ensure arrays have the same shape | ||
| # probably needs to throw an exception | ||
| if lat.shape != lon.shape: | ||
| return np.zeros_like(lat, dtype=bool) |
There was a problem hiding this comment.
All these checks should be unneccesary as the data container guarntees these things.
| # Convert inputs to NumPy arrays, preserving masked arrays | ||
| try: | ||
| lat = np.asarray(lat, dtype=float) | ||
| lon = np.asarray(lon, dtype=float) | ||
| except (ValueError, TypeError): | ||
| return np.zeros_like(lat, dtype=bool) |
There was a problem hiding this comment.
Why? ma masked arrays ARE numpy arrays. I would not do this...
There was a problem hiding this comment.
This code deletes the masks... missing values are already indicated in the mask.
| mask &= (lon >= 0) & (lon <= 360) | ||
|
|
||
| # Ensure no NaN in longitude | ||
| mask &= ~np.isnan(lon) |
|
|
||
|
|
||
|
|
||
| def clean_lat_lon(lat, lon): |
| ) | ||
|
|
||
|
|
||
| class Bufr2iodaConfig: |
There was a problem hiding this comment.
Should probably be GdasAppConfig.
There was a problem hiding this comment.
needs to be cleaned; contains unnecessary code
| # print("BBBBBBBBBBBBBBBBBBB") | ||
| # print(buoy_mask) | ||
| # print(temp_mask.size) | ||
| # print(buoy_type.size) | ||
| # print(buoy_mask.size) | ||
| # print(np.count_nonzero(buoy_mask)) | ||
| # print(np.count_nonzero(temp_mask)) | ||
| # print("BBBBBBBBBBBBBBBBBBB") | ||
| # container.apply_mask(buoy_mask & temp_mask) |
There was a problem hiding this comment.
Please remove commented out code.
|
|
||
| # rpid = stationID: string array (e.g., 'A8xxx') | ||
| # buoy_type: int array (e.g., 1, 2, 3), etc. | ||
| drifter_mask = np.isin(buoy_type, drifter_buoy_types, assume_unique=True) |
There was a problem hiding this comment.
Usage of the assume_unique=True flag is not recommended here as bouy_type array will definitely have duplicate values. Under certain circumstances this can lead to unexpected results.
|
|
||
| # Handle masked (missing) BUYT values | ||
| # If BUYT is masked, assume not a drifter unless RPID suggests otherwise | ||
| drifter_mask = np.where(buoy_type.mask, rpid_drifter_mask, drifter_mask) |
There was a problem hiding this comment.
Should this be drifter_mask = drifter_mask & ~bouy_type.mask, basically mask out the values when the bouy_type mask is a "missing" element.
| print(np.count_nonzero(buoy_mask)) | ||
| print(np.count_nonzero(temp_mask)) | ||
| print("BBBBBBBBBBBBBBBBBBB") | ||
| # container.apply_mask(buoy_mask & temp_mask) |
There was a problem hiding this comment.
Please delete debug print statements
| from dataclasses import dataclass | ||
|
|
||
|
|
||
| OCEAN_BASIN_FILE = "/work/noaa/global/glopara/fix/gdas/soca/20240802/common/RECCAP2_region_masks_all_v20221025.nc" |
There was a problem hiding this comment.
Hard coded path string... this will only work on Orion and Hercules HPCs...
I think the right way to handle this is to add the project "https://github.com/RECCAP2-ocean/R2-shared-resources" as a submodule to this project. This way it would be platform independent...
|
DId you mean to check in "spoc-tests.yaml"? |
|
Please remove code related to testing from this branch. |
3 participants
