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

Mhs/das 2216/quick fixes #19

Merged
merged 25 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
bb7ebd8
DAS-1934: Most basic implementation to allow consistent grids
flamingbear Aug 28, 2024
d072bca
DAS-1934: Update comments and README for new grid validation.
flamingbear Aug 29, 2024
addf305
DAS-1934: Update changelog
flamingbear Aug 29, 2024
8150470
DAS-1934: Update changelog redux
flamingbear Aug 29, 2024
44589d8
DAS-1934: Update service version and address snyk vulnerabilities
flamingbear Aug 29, 2024
f9890f0
Update CHANGELOG.md
flamingbear Sep 1, 2024
ed0ff0e
DAS-1934: Fix release note extraction script.
flamingbear Sep 1, 2024
7622aa7
DAS-2216: Quick Fixes 1, 2 and 4
flamingbear Sep 9, 2024
b319a63
Merge remote-tracking branch 'origin/main' into mhs/DAS-2216/quick-fixes
flamingbear Sep 12, 2024
31c5bea
Merge branch 'main' into mhs/DAS-2216/quick-fixes
joeyschultz Sep 23, 2024
2a5a6ef
DAS-2216: Modify earthdata-varinfo config for quick fix 1
joeyschultz Oct 10, 2024
4477475
DAS-2216: Resolve unit tests that were failing due to quick fixes
joeyschultz Oct 10, 2024
404088c
DAS-2216: Update service version and CHANGELOG.
joeyschultz Oct 10, 2024
72223b9
Modify transpose_if_xdim_less_than_ydim to resolve mask array not bei…
joeyschultz Oct 16, 2024
656c858
Create notebook for PR testing and demo purposes
joeyschultz Oct 16, 2024
3601f92
Install and run pre-commit
joeyschultz Oct 16, 2024
735894d
Remove unused exception, fix CHANGELOG links, and other minor updates
joeyschultz Oct 21, 2024
8770da0
Reorganize some of the notebook functions
joeyschultz Oct 21, 2024
78dd9df
Modify assumption comments in get_variable_values for clarity
joeyschultz Oct 21, 2024
25cdc10
Add to varinfo config, re-enable MissingReprojectedDataError, modify …
joeyschultz Oct 23, 2024
b707bd2
Remove TEMPO_O3TOT_L2_example.ipynb and add it to JIRA ticket instead.
joeyschultz Oct 23, 2024
6678d5a
Simplify variable transposal, update effected typehints and unit tests
joeyschultz Oct 25, 2024
e5d4be1
Merge remote-tracking branch 'origin/main' into mhs/DAS-2216/quick-fixes
joeyschultz Oct 25, 2024
bde29b6
Add get_rows_per_scan utility and associated unit tests
joeyschultz Oct 28, 2024
cd54588
Apply coordinates MetadataOverride to geolocation group
joeyschultz Oct 28, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 17 additions & 7 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
# Changelog

## [v1.2.0] - 2024-10-10

### Changed

- [[DAS-2216](https://bugs.earthdata.nasa.gov/browse/DAS-2216)]
The Swath Projector has been updated with quick fixes to add support for TEMPO level 2 data.
owenlittlejohns marked this conversation as resolved.
Show resolved Hide resolved

## [v1.1.1] - 2024-09-16

### Changed

- [[TRT-558](https://bugs.earthdata.nasa.gov/browse/TRT-558)]
Expand All @@ -12,6 +20,7 @@
also been renamed to `earthdata_varinfo_config.json`.

## [v1.1.0] - 2024-08-29

### Changed

- [[DAS-1934](https://bugs.earthdata.nasa.gov/browse/DAS-1934)]
Expand All @@ -37,14 +46,15 @@ include updated documentation and files outlined by the

Repository structure changes include:

* Migrating `pymods` directory to `swath_projector`.
* Migrating `swotrepr.py` to `swath_projector/adapter.py`.
* Addition of `swath_projector/main.py`.
- Migrating `pymods` directory to `swath_projector`.
- Migrating `swotrepr.py` to `swath_projector/adapter.py`.
- Addition of `swath_projector/main.py`.

For more information on internal releases prior to NASA open-source approval,
see legacy-CHANGELOG.md.

[v1.1.1]:(https://github.com/nasa/harmony-swath-projector/releases/tag/1.1.0)
[v1.1.0]:(https://github.com/nasa/harmony-swath-projector/releases/tag/1.0.1)
[v1.0.1]:(https://github.com/nasa/harmony-swath-projector/releases/tag/1.0.1)
[v1.0.0]:(https://github.com/nasa/harmony-swath-projector/releases/tag/1.0.0)
[v1.2.0]: (https://github.com/nasa/harmony-swath-projector/releases/tag/1.2.0)
[v1.1.1]: (https://github.com/nasa/harmony-swath-projector/releases/tag/1.1.1)
[v1.1.0]: (https://github.com/nasa/harmony-swath-projector/releases/tag/1.1.0)
[v1.0.1]: (https://github.com/nasa/harmony-swath-projector/releases/tag/1.0.1)
[v1.0.0]: (https://github.com/nasa/harmony-swath-projector/releases/tag/1.0.0)
6 changes: 3 additions & 3 deletions bin/project_local_granule.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ def project_granule(
{
lyonthefrog marked this conversation as resolved.
Show resolved Hide resolved
'url': local_file_path,
'temporal': {
'start': '2021-01-03T23:45:00.000Z',
'end': '2020-01-04T00:00:00.000Z',
'start': '2020-01-03T23:45:00.000Z',
'end': '2025-01-04T00:00:00.000Z',
},
'bbox': [-180, -90, 180, 90],
}
Expand All @@ -141,5 +141,5 @@ def project_granule(

reprojector = SwathProjectorAdapter(message, config=config(False))

with patch('swotrepr.shutil.rmtree', side_effect=rmtree_side_effect):
with patch('swath_projector.adapter.shutil.rmtree', side_effect=rmtree_side_effect):
owenlittlejohns marked this conversation as resolved.
Show resolved Hide resolved
reprojector.invoke()
2 changes: 1 addition & 1 deletion docker/service_version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.1.1
1.2.0
43 changes: 40 additions & 3 deletions swath_projector/earthdata_varinfo_config.json
Original file line number Diff line number Diff line change
@@ -1,12 +1,35 @@
{
"Identification": "Swath Projector VarInfo configuration",
"Version": 3,
"Version": 4,
"CollectionShortNamePath": [
"ShortName"
"ShortName",
"collection_shortname"
],
"Mission": {
"VNP10": "VIIRS"
"VNP10": "VIIRS",
"TEMPO_O3TOT_L2": "TEMPO"
},
"ExcludedScienceVariables": [
{
"Applicability": {
"Mission": "TEMPO",
"ShortNamePath": "TEMPO_O3TOT_L2"
},
"VariablePattern": [
"/support_data/a_priori_layer_o3",
"/support_data/cal_adjustment",
"/support_data/dNdR",
"/support_data/layer_efficiency",
"/support_data/lut_wavelength",
"/support_data/N_value",
"/support_data/N_value_residual",
"/support_data/ozone_sensitivity_ratio",
"/support_data/step_1_N_value_residual",
"/support_data/step_2_N_value_residual",
"/support_data/temp_sensitivity_ratio"
]
}
],
owenlittlejohns marked this conversation as resolved.
Show resolved Hide resolved
"MetadataOverrides": [
{
"Applicability": {
Expand All @@ -21,6 +44,20 @@
}
],
"_Description": "VNP10 SnowData variables have incorrect relative paths for coordinates."
},
{
"Applicability": {
"Mission": "TEMPO",
"ShortNamePath": "TEMPO_O3TOT_L2",
"VariablePattern": "^/product/.*|^/support_data/.*"
},
"Attributes": [
{
"Name": "coordinates",
"Value": "/geolocation/latitude, /geolocation/longitude"
lyonthefrog marked this conversation as resolved.
Show resolved Hide resolved
}
],
"_Description": "TEMPO_O3TOT_L2 variables only contain basenames for coordinates, which are found in sibling hierarchical groups. This rule fully qualifies the paths to these coordinates."
owenlittlejohns marked this conversation as resolved.
Show resolved Hide resolved
}
]
}
1 change: 1 addition & 0 deletions swath_projector/interpolation.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ def get_ewa_results(
ewa_information['target_area'],
variable['values'],
maximum_weight_mode=maximum_weight_mode,
rows_per_scan=2, # Added in QuickFix DAS-2216 to be fixed in DAS-2220
)

if variable['fill_value'] is not None:
Expand Down
42 changes: 36 additions & 6 deletions swath_projector/utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ def get_variable_values(
As the variable data are returned as a `numpy.ma.MaskedArray`, the will
return no data in the filled pixels. To ensure that the data are
correctly handled, the fill value is applied to masked pixels using the
`filled` method.
`filled` method. The variable values are transposed if the `along-track`
dimension size is less than the `across-track` dimension size.

"""
# TODO: Remove in favour of apply2D or process_subdimension.
Expand All @@ -42,11 +43,15 @@ def get_variable_values(
if len(variable[:].shape) == 1:
return make_array_two_dimensional(variable[:])
elif 'time' in input_file.variables and 'time' in variable.dimensions:
# Assumption: Array = (1, y, x)
return variable[0][:].filled(fill_value=fill_value)
# Assumption: Array = (time, along-track, across-track)
owenlittlejohns marked this conversation as resolved.
Show resolved Hide resolved
return transpose_if_xdim_less_than_ydim(
variable[0][:].filled(fill_value=fill_value)
)
else:
# Assumption: Array = (y, x)
return variable[:].filled(fill_value=fill_value)
# Assumption: Array = (along-track, across-track)
return transpose_if_xdim_less_than_ydim(
Copy link
Member

Choose a reason for hiding this comment

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

I have no idea if that assumption is still correct, same with L45

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is a reasonable assumption (transpose to get xdim < ydim) as a default behavior. Follow-up ticket on this point is allow for a configuration entry to explicitly define the along-track vs across-track dimension. In that ticket I am proposing a swath feature metadata variable, similar to grid_mapping, that provides this definition - trying to move towards convergible metadata standards as discussed in the convergence tiger team.

Copy link
Member

@flamingbear flamingbear Oct 18, 2024

Choose a reason for hiding this comment

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

I was talking about the line that says and if it's not true, it should be removed

        # Assumption: Array = (y, x)

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps clarify the comment to "First assume Array = (along-track, across-track), but transpose if along-track.size < across-track.size. Similarly for line 45.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To keep it more concise, I thought it would be best to modify the comments to:
line 45: # Assumption: Array = (time, along-track, across-track)
line 50: # Assumption: Array = (along-track, across-track)

Fixed in commit 78dd9df

variable[:].filled(fill_value=fill_value)
)


def get_coordinate_variable(
Expand All @@ -62,7 +67,12 @@ def get_coordinate_variable(
if coordinate_substring in coordinate.split('/')[-1] and variable_in_dataset(
coordinate, dataset
):
return dataset[coordinate]
# QuickFix (DAS-2216) for short and wide swaths
Copy link
Member

Choose a reason for hiding this comment

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

Is this comment in the right place still? They are in the code so that they can be removed when the quick fixes are actual fixes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Lines 70-73 are all part of the quickfix so I think the comment is where we want it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm actually on the fence about this as I suspect these are essentially default behaviors and not necessarily going away with the quick fix. That said, there is at least the case of setting rows-per-scan = 2 that will need follow-up modification. We can go with it for now, but do need to follow-up.

if dataset[coordinate].ndim == 1:
return dataset[coordinate]

return transpose_if_xdim_less_than_ydim(dataset[coordinate])

raise MissingCoordinatesError(coordinates_tuple)


Expand Down Expand Up @@ -216,3 +226,23 @@ def make_array_two_dimensional(one_dimensional_array: np.ndarray) -> np.ndarray:

"""
return np.expand_dims(one_dimensional_array, 1)


def transpose_if_xdim_less_than_ydim(variable: np.ndarray) -> np.ndarray:
"""Return transposed variable when variable is wider than tall.

QuickFix (DAS-2216): We presume that a swath has more rows than columns and
if that's not the case we transpose it so that it does.
"""
if variable.ndim != 2:
raise ValueError(
f'Input variable must be 2 dimensional, but got {variable.ndim} dimensions.'
)
if variable.shape[0] < variable.shape[1]:
transposed_variable = np.ma.transpose(variable).copy()
if hasattr(variable, 'mask'):
lyonthefrog marked this conversation as resolved.
Show resolved Hide resolved
transposed_variable.mask = np.ma.transpose(variable[:].mask)
owenlittlejohns marked this conversation as resolved.
Show resolved Hide resolved

return transposed_variable

return variable
5 changes: 5 additions & 0 deletions tests/unit/test_interpolation.py
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ def test_resample_ewa(
self.mock_target_area,
mock_values,
maximum_weight_mode=False,
rows_per_scan=2, # Added in QuickFix DAS-2216 to be fixed in DAS-2220
)
mock_write_output.assert_called_once_with(
self.mock_target_area,
Expand Down Expand Up @@ -423,6 +424,7 @@ def test_resample_ewa(
self.mock_target_area,
mock_values,
maximum_weight_mode=False,
rows_per_scan=2, # Added in QuickFix DAS-2216 to be fixed in DAS-2220
)
mock_write_output.assert_called_once_with(
self.mock_target_area,
Expand Down Expand Up @@ -491,6 +493,7 @@ def test_resample_ewa_nn(
self.mock_target_area,
mock_values,
maximum_weight_mode=True,
rows_per_scan=2, # Added in QuickFix DAS-2216 to be fixed in DAS-2220
)
mock_write_output.assert_called_once_with(
self.mock_target_area,
Expand Down Expand Up @@ -530,6 +533,7 @@ def test_resample_ewa_nn(
self.mock_target_area,
mock_values,
maximum_weight_mode=True,
rows_per_scan=2, # Added in QuickFix DAS-2216 to be fixed in DAS-2220
)
mock_write_output.assert_called_once_with(
self.mock_target_area,
Expand Down Expand Up @@ -581,6 +585,7 @@ def test_resample_ewa_nn(
harmony_target_area,
mock_values,
maximum_weight_mode=True,
rows_per_scan=2, # Added in QuickFix DAS-2216 to be fixed in DAS-2220
)

# The Harmony target area should be given to the output function
Expand Down
55 changes: 54 additions & 1 deletion tests/unit/test_utilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
get_variable_values,
make_array_two_dimensional,
qualify_reference,
transpose_if_xdim_less_than_ydim,
variable_in_dataset,
)

Expand Down Expand Up @@ -79,7 +80,9 @@ def test_get_variable_values(self):
wind_speed_values = get_variable_values(dataset, wind_speed, None)
self.assertIsInstance(wind_speed_values, np.ndarray)
self.assertEqual(len(wind_speed_values.shape), 2)
self.assertEqual(wind_speed_values.shape, wind_speed.shape)
self.assertEqual(
wind_speed_values.shape, np.ma.transpose(wind_speed).shape
lyonthefrog marked this conversation as resolved.
Show resolved Hide resolved
)

with self.subTest('Masked values are set to fill value.'):
fill_value = 210
Expand Down Expand Up @@ -358,3 +361,53 @@ def test_make_array_two_dimensional(self):

self.assertEqual(len(output_array.shape), 2)
np.testing.assert_array_equal(output_array, expected_output)


class TestTransposeIfXdimLessThanYdim(TestCase):
owenlittlejohns marked this conversation as resolved.
Show resolved Hide resolved

def test_wider_than_tall(self):
"""Test case where x dim < y dim and should transpose."""
input_array = np.ma.array([[1, 2, 3], [4, 5, 6]])
expected_output = np.ma.array([[1, 4], [2, 5], [3, 6]])
owenlittlejohns marked this conversation as resolved.
Show resolved Hide resolved
result = transpose_if_xdim_less_than_ydim(input_array)
np.testing.assert_array_equal(result, expected_output)
self.assertEqual(result.shape, (3, 2))

def test_taller_than_wide(self):
"""Test case where x < y and should not transpose."""
input_array = np.ma.array([[1, 2], [3, 4], [5, 6]])
result = transpose_if_xdim_less_than_ydim(input_array)
np.testing.assert_array_equal(result, input_array)
self.assertEqual(result.shape, (3, 2))

def test_square_array(self):
"""Test case where y dim == x dim and should not transpose."""
input_array = np.ma.array([[1, 2], [3, 4]])
result = transpose_if_xdim_less_than_ydim(input_array)
np.testing.assert_array_equal(result, input_array)
self.assertEqual(result.shape, (2, 2))

def test_1d_array(self):
"""Test case with a 1D array"""
input_array = np.ma.array([1, 2, 3])
with self.assertRaisesRegex(ValueError, 'variable must be 2 dimensional'):
owenlittlejohns marked this conversation as resolved.
Show resolved Hide resolved
transpose_if_xdim_less_than_ydim(input_array)

def test_3d_array(self):
"""Test case with a 3D array"""
input_array = np.ma.array([[[1, 2], [3, 4]], [[5, 6], [7, 8]]])
with self.assertRaisesRegex(ValueError, 'variable must be 2 dimensional'):
owenlittlejohns marked this conversation as resolved.
Show resolved Hide resolved
transpose_if_xdim_less_than_ydim(input_array)

def test_masked_array(self):
"""Test case with a masked array"""
input_array = np.ma.array(
[[1, 2, 3], [4, 5, 6]], mask=[[True, False, False], [False, True, False]]
)
expected_output = np.ma.array(
[[1, 4], [2, 5], [3, 6]],
mask=[[True, False], [False, True], [False, False]],
)
result = transpose_if_xdim_less_than_ydim(input_array)
np.testing.assert_array_equal(result, expected_output)
np.testing.assert_array_equal(result.mask, expected_output.mask)
Loading