Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DAS-2270 - Check for dimension ordering done for SMAP L3 2d variables #19

Draft
wants to merge 22 commits into
base: DAS-2208-SMAP-L3
Choose a base branch
from

Conversation

sudha-murthy
Copy link
Collaborator

@sudha-murthy sudha-murthy commented Nov 22, 2024

Description

This PR includes changes to determine the right dimension order when doing spatial subsets for SMAP L3. The
latitude and longitude coordinates are used to make the determination. SMAP L3 does not have dimension scales and is not CF compliant.
This PR does not include tests for multiple grids which is part of the ticket. which will be a separate commit

Jira Issue ID

DAS-2270

Local Test Steps

  • The unit tests to determine the dimensions order should pass
  • For multiple grid test - and end-end test with AM and PM variables should include the index ranges for coordinates from
    both AM and PM groups

http://localhost:3000/C1268452378-EEDTEST/ogc-api-coverages/1.0.0/collections/parameter_vars/coverage/rangeset?forceAsync=true&granuleId=G1268452388-EEDTEST&subset=lat(54%3A72)&subset=lon(2%3A42)&format=application%2Fx-netcdf4&variable=Soil_Moisture_Retrieval_Data_AM%2Fsurface_flag%2CSoil_Moisture_Retrieval_Data_PM%2Fsurface_flag_pm&skipPreview=true

Should show the following in the log:
variables_with_ranges: /Soil_Moisture_Retrieval_Data_PM/surface_flag_pm[9:38][487:594], /Soil_Moisture_Retrieval_Data_AM/surface_flag[9:38][487:594], /Soil_Moisture_Retrieval_Data_AM/longitude[9:38][487:594], /Soil_Moisture_Retrieval_Data_PM/longitude_pm[9:38][487:594], /Soil_Moisture_Retrieval_Data_AM/latitude[9:38][487:594], /Soil_Moisture_Retrieval_Data_PM/latitude_pm[9:38][487:594]

PR Acceptance Checklist

  • Jira ticket acceptance criteria met.
  • CHANGELOG.md updated to include high level summary of PR changes.
  • docker/service_version.txt updated if publishing a release.
  • [X ] Tests added/updated and passing.
  • Documentation updated (if needed).

dimension_indices: list,
dimension_size: int,
index_for_dimension: int,
x_or_y_index: int,
Copy link

@D-Auty D-Auty Nov 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

index_for_dimension => use_row_not_col (boolean)
x_or_y_index => dim_order_is_y_x (boolean)
Invocations should use parameter name when providing direct values, e.g., use_row_not_col=False
And implementation should convert booleans into array indices, perhaps with comments to clarify.
This will hopefully make things a bit clearer.

Also, possibly, this routine should get the dimension_names, rather than passing this list around. But I realize that may not make sense given the parameters required for getting the dimension_names.

lat_arr, lon_arr, latitude_coordinate, longitude_coordinate
)

y_x_order, projected_y = get_dimension_order(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dim_order_is_y_x, projected_dim_name = (

)
# if is row, the index_for_dimension is 0
y_dim = get_dimension_array_from_geo_points(
lat_arr, lon_arr, crs, row_indices, row_size, 0, y_x_order
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lat_arr, lon_arr, crs, row_indices, row_size, dim_order_is_y_x, use_row_not_col=False

lat_arr, lon_arr, crs, row_indices, row_size, 0, y_x_order
)

x_y_order, projected_x = get_dimension_order(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dim_order_is_y_x, projected_dim_name = (

)
# if is column, the index_for_dimension is 1
x_dim = get_dimension_array_from_geo_points(
lat_arr, lon_arr, crs, col_indices, col_size, 1, x_y_order
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lat_arr, lon_arr, crs, row_indices, row_size, dim_order_is_y_x, use_row_not_col=True

# and call it y
if is_row is True and np.all(np.diff(lat_arr_values) != 0):
return (1, projected_y)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should be using not is_close for floating point, rather than an absolute != 0? (and in following statements)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

np.isclose is for comparing two arrays I think. will check into it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will change it to np.allclose(lat_arr_values, lat_arr_values[0])

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants