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.

Sign up for GitHub

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

Test fixes #154

Merged
merged 12 commits into from
Mar 4, 2025
Merged

Test fixes #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
dbe6279
set minimum scikit-learn version and use root_mean_squared_error
ejm714 Mar 3, 2025
56d2e23
sentinel-2 data has been reprocessed leading to changes: part 1 -- te…
ejm714 Mar 3, 2025
d997368
sentinel-2 data has been reprocessed leading to changes: part 2 -- ex…
ejm714 Mar 3, 2025
54e9c65
avoid writing out git hash to config yaml so it does not change with …
ejm714 Mar 3, 2025
80b9d62
be more flexible with scikit-learn
ejm714 Mar 3, 2025
cc6417b
override save path in experiment test to use tmp_path
ejm714 Mar 3, 2025
c3347fa
remove ceiling for matplotlib and remove duplicate clean command
ejm714 Mar 3, 2025
19d9ef4
update test assets with latest matplotlib
ejm714 Mar 3, 2025
38ffda0
address pandas warnings
ejm714 Mar 3, 2025
0cb6efd
fix test from writing out to assets
ejm714 Mar 4, 2025
72576dd
latest hash
ejm714 Mar 4, 2025
d9e91ad
make assets should be runnable again if it failed halfway through
ejm714 Mar 4, 2025
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
11 changes: 3 additions & 8 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,8 @@ PYTHON_INTERPRETER = python
# COMMANDS #
#################################################################################

clean:
rm -f .coverage
rm -f coverage.xml
rm -fr htmlcov/
rm -fr .pytest_cache
## Remove all build, test, coverage and Python artifacts
clean: clean-build clean-pyc clean-test

## Set up python interpreter environment
create_environment:
Expand All @@ -42,7 +39,7 @@ test: clean lint

## Make assets
assets:
rm -r tests/assets/experiment
rm -fr tests/assets/experiment
python cyfi/experiment.py tests/assets/experiment_config.yaml

docs: ## build the static version of the docs
Expand All @@ -53,8 +50,6 @@ docs: ## build the static version of the docs
docs-serve: ## serve documentation to livereload while you work
cd docs && mkdocs serve

clean: clean-build clean-pyc clean-test ## remove all build, test, coverage and Python artifacts

clean-build: ## remove build artifacts
rm -fr build/
rm -fr dist/
Expand Down
12 changes: 7 additions & 5 deletions cyfi/evaluate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from sklearn.metrics import (
classification_report,
mean_absolute_error,
mean_squared_error,
root_mean_squared_error,
r2_score,
)
from zipfile import ZipFile
Expand Down Expand Up @@ -194,23 +194,23 @@ def __init__(
@staticmethod
def calculate_severity_metrics(y_true, y_pred, region=None):
results = dict()
results["overall_rmse"] = mean_squared_error(y_true, y_pred, squared=False)
results["overall_rmse"] = root_mean_squared_error(y_true, y_pred)
results["overall_mae"] = mean_absolute_error(y_true, y_pred)

if region is not None:
df = pd.concat([y_true, y_pred, region], axis=1)
df.columns = ["y_true", "y_pred", "region"]
results["regional_rmse"] = (
df.groupby("region")
.apply(lambda x: mean_squared_error(x.y_true, x.y_pred, squared=False))
.apply(lambda x: root_mean_squared_error(x.y_true, x.y_pred), include_groups=False)
.to_dict()
)
results["region_averaged_rmse"] = np.mean(
[val for val in results["regional_rmse"].values()]
)
results["regional_mae"] = (
df.groupby("region")
.apply(lambda x: mean_absolute_error(x.y_true, x.y_pred))
.apply(lambda x: mean_absolute_error(x.y_true, x.y_pred), include_groups=False)
.to_dict()
)

Expand All @@ -235,7 +235,9 @@ def calculate_log_density_metrics(y_true_df, y_pred_df, region=None):
df = pd.concat([y_true, y_pred, region], axis=1)
df.columns = ["y_true", "y_pred", "region"]
results["regional_r_squared"] = (
df.groupby("region").apply(lambda x: r2_score(x.y_true, x.y_pred)).to_dict()
df.groupby("region")
.apply(lambda x: r2_score(x.y_true, x.y_pred), include_groups=False)
.to_dict()
)

return results
Expand Down
4 changes: 2 additions & 2 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ dependencies = [
"gradio",
"lightgbm",
"loguru",
"matplotlib<3.8.0",
"matplotlib",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we no longer need to enforce a version of matplotlib?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There was a mpl bug that was the cause of this ceiling (e379dec) but I can't remember the specifics. In general, we only want floors not ceilings for packages. Ceilings are often a temporary workaround and it looked like this one was outdated (indeed as it was given tests pass with later versions)

"numpy",
"odc-stac",
"opencv-python",
Expand All @@ -45,7 +45,7 @@ dependencies = [
"pydantic>=2.0",
"pystac",
"pystac-client",
"scikit-learn",
"scikit-learn>=1.4",
"seaborn",
"repro-zipfile",
"rioxarray",
Expand Down
4 changes: 2 additions & 2 deletions tests/assets/experiment/config_artifact.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ cyfi_model_config:
params:
application: regression
early_stopping_round: 100
feature_fraction: 1.0
feature_fraction: 0.6
learning_rate: 0.1
max_depth: -1
metric: rmse
Expand All @@ -32,7 +32,7 @@ features_config:
use_sentinel_bands:
- B02
- SCL
last_commit_hash: ea440f57d459c98cf7cc03f7e24a0df05c5e23d2
last_commit_hash: 0cb6efd14aebfb44740df7d8aabda3ff768bbfff
predict_csv: tests/assets/evaluate_data.csv
save_dir: tests/assets/experiment
train_csv: tests/assets/train_data.csv
8 changes: 4 additions & 4 deletions tests/assets/experiment/features_test.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
sample_id,B02_min,B02_mean,B02_max,month,days_before_sample
494fb91d1fb8697e73b90e5d0c9420ed,160.0,2195.007438894793,7144.0,8,8
a9af79990527a83e31f9a954d3e334d5,95.0,791.3992941009542,2530.0,7,3
d823c957bedf702bb1dd8d562e45a7b8,53.0,243.86791970912589,6584.0,7,15
f79c66af395e5de8fb3c5cc1b1ed6993,151.0,422.4892456359102,4228.0,5,0
494fb91d1fb8697e73b90e5d0c9420ed,79.0,2037.4023722627737,7144.0,8,8
a9af79990527a83e31f9a954d3e334d5,95.0,790.1282817918053,2530.0,7,3
d823c957bedf702bb1dd8d562e45a7b8,141.0,714.5378532093528,8680.0,7,12
f79c66af395e5de8fb3c5cc1b1ed6993,151.0,420.10597536870915,3104.0,5,0
10 changes: 5 additions & 5 deletions tests/assets/experiment/features_train.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
sample_id,B02_min,B02_mean,B02_max,month,days_before_sample
1969e5e476b5971a377c268c7a8a9ca3,130.0,1040.891355140187,7088.0,7,3
3a2c48812b551d720f8d56772efa6df1,297.0,902.58031496063,4062.0,8,24
6696747608f3d2f469b3e3c28ef9866d,137.0,1274.4461713419257,8416.0,7,13
9c601f226c2af07d570134127a7fda27,300.0,1037.5978647686832,3272.0,7,29
ef04be46891bd8bf9a9beab01aa74b4f,46.0,396.00851466414383,4208.0,6,23
1969e5e476b5971a377c268c7a8a9ca3,130.0,1036.9352401511064,7088.0,7,3
3a2c48812b551d720f8d56772efa6df1,297.0,911.2863705972435,4062.0,8,24
6696747608f3d2f469b3e3c28ef9866d,137.0,1218.241975308642,8232.0,7,13
9c601f226c2af07d570134127a7fda27,301.0,1036.5135492399206,5000.0,7,29
ef04be46891bd8bf9a9beab01aa74b4f,50.0,239.37847344354554,1876.0,6,13
Binary file modified tests/assets/experiment/metrics/actual_density_boxplot.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/assets/experiment/metrics/crosstab.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/assets/experiment/metrics/density_kde.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/assets/experiment/metrics/density_scatterplot.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified tests/assets/experiment/model.zip
View file
Open in desktop
Binary file not shown.
8 changes: 4 additions & 4 deletions tests/assets/experiment/sentinel_metadata_test.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
sample_id,item_id,cloud_pct,num_water_pixels,days_before_sample,visual_href
494fb91d1fb8697e73b90e5d0c9420ed,S2A_MSIL2A_20190818T170851_R112_T15SUD_20201005T015156,0.010019129603060737,941.0,8,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/15/S/UD/2019/08/18/S2A_MSIL2A_20190818T170851_N0212_R112_T15SUD_20201005T015156.SAFE/GRANULE/L2A_T15SUD_A021702_20190818T171838/IMG_DATA/R10m/T15SUD_20190818T170851_TCI_10m.tif
a9af79990527a83e31f9a954d3e334d5,S2B_MSIL2A_20180708T155819_R097_T17SPV_20201011T133356,0.004069006543723974,56382.0,3,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/17/S/PV/2018/07/08/S2B_MSIL2A_20180708T155819_N0212_R097_T17SPV_20201011T133356.SAFE/GRANULE/L2A_T17SPV_A006987_20180708T161248/IMG_DATA/R10m/T17SPV_20180708T155819_TCI_10m.tif
d823c957bedf702bb1dd8d562e45a7b8,S2B_MSIL2A_20170713T173909_R098_T13TDE_20210210T083500,0.014554095195663475,66558.0,15,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/13/T/DE/2017/07/13/S2B_MSIL2A_20170713T173909_N0212_R098_T13TDE_20210210T083500.SAFE/GRANULE/L2A_T13TDE_A001840_20170713T174714/IMG_DATA/R10m/T13TDE_20170713T173909_TCI_10m.tif
f79c66af395e5de8fb3c5cc1b1ed6993,S2A_MSIL2A_20180521T154911_R054_T18TUK_20201012T122951,0.0155145746579417,6416.0,0,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/18/T/UK/2018/05/21/S2A_MSIL2A_20180521T154911_N0212_R054_T18TUK_20201012T122951.SAFE/GRANULE/L2A_T18TUK_A015209_20180521T155356/IMG_DATA/R10m/T18TUK_20180521T154911_TCI_10m.tif
494fb91d1fb8697e73b90e5d0c9420ed,S2A_MSIL2A_20190818T170851_R112_T15SUD_20201005T015156,0.010019129603060737,1096.0,8,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/15/S/UD/2019/08/18/S2A_MSIL2A_20190818T170851_N0212_R112_T15SUD_20201005T015156.SAFE/GRANULE/L2A_T15SUD_A021702_20190818T171838/IMG_DATA/R10m/T15SUD_20190818T170851_TCI_10m.tif
Copy link
Collaborator

Choose a reason for hiding this comment

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

When I rerun make test on this branch, I get different values for num_water_pixels. Mine generally align with the previous version of this file. I'm not sure why mine is different -- I started with a clean environment and re-installed the requirements.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm also getting slightly different values for features_test.csv and features_train.csv

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmmm that's very curious and good to know

Copy link
Collaborator Author

@ejm714 ejm714 Mar 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

after a long investigation, the number of water pixel changes (with cascading effects on band summary stats) appear to be coming from OpenCV (due to variations in underlying hardware, operating systems, and library implementations).

Different processors and architectures may handle floating-point calculations with slight variations, leading to minor differences in the computed pixel values during interpolation.

this is the relevant part of the code

cyfi/cyfi/data/features.py

Lines 165 to 169 in 85c82ca

if config.filter_to_water_area:
if band != "SCL":
scaled_scl = cv2.resize(scl_array[0], (arr.shape[2], arr.shape[1]))
arr = arr[0][scaled_scl == 6]
sample_item_features["num_water_pixels"] = arr.size

a9af79990527a83e31f9a954d3e334d5,S2B_MSIL2A_20180708T155819_R097_T17SPV_20201011T133356,0.004069006543723974,56524.0,3,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/17/S/PV/2018/07/08/S2B_MSIL2A_20180708T155819_N0212_R097_T17SPV_20201011T133356.SAFE/GRANULE/L2A_T17SPV_A006987_20180708T161248/IMG_DATA/R10m/T17SPV_20180708T155819_TCI_10m.tif
d823c957bedf702bb1dd8d562e45a7b8,S2B_MSIL2A_20170716T174909_R141_T13TDE_20210210T100624,0.026806267171604663,55385.0,12,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/13/T/DE/2017/07/16/S2B_MSIL2A_20170716T174909_N0212_R141_T13TDE_20210210T100624.SAFE/GRANULE/L2A_T13TDE_A001883_20170716T175124/IMG_DATA/R10m/T13TDE_20170716T174909_TCI_10m.tif
f79c66af395e5de8fb3c5cc1b1ed6993,S2A_MSIL2A_20180521T154911_R054_T18TUK_20201012T122951,0.0155145746579417,6577.0,0,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/18/T/UK/2018/05/21/S2A_MSIL2A_20180521T154911_N0212_R054_T18TUK_20201012T122951.SAFE/GRANULE/L2A_T18TUK_A015209_20180521T155356/IMG_DATA/R10m/T18TUK_20180521T154911_TCI_10m.tif
10 changes: 5 additions & 5 deletions tests/assets/experiment/sentinel_metadata_train.csv
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
sample_id,item_id,cloud_pct,num_water_pixels,days_before_sample,visual_href
1969e5e476b5971a377c268c7a8a9ca3,S2B_MSIL2A_20200708T184919_R113_T10SEH_20200912T124306,0.014730029943011688,1712.0,3,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/10/S/EH/2020/07/08/S2B_MSIL2A_20200708T184919_N0212_R113_T10SEH_20200912T124306.SAFE/GRANULE/L2A_T10SEH_A017442_20200708T185408/IMG_DATA/R10m/T10SEH_20200708T184919_TCI_10m.tif
3a2c48812b551d720f8d56772efa6df1,S2A_MSIL2A_20190804T154911_R054_T18TVL_20201004T201836,0.03421013743165361,3810.0,24,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/18/T/VL/2019/08/04/S2A_MSIL2A_20190804T154911_N0212_R054_T18TVL_20201004T201836.SAFE/GRANULE/L2A_T18TVL_A021501_20190804T160118/IMG_DATA/R10m/T18TVL_20190804T154911_TCI_10m.tif
6696747608f3d2f469b3e3c28ef9866d,S2A_MSIL2A_20200712T155911_R097_T17SPV_20200912T223337,0.030148720999405115,2638.0,13,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/17/S/PV/2020/07/12/S2A_MSIL2A_20200712T155911_N0212_R097_T17SPV_20200912T223337.SAFE/GRANULE/L2A_T17SPV_A026406_20200712T161123/IMG_DATA/R10m/T17SPV_20200712T155911_TCI_10m.tif
9c601f226c2af07d570134127a7fda27,S2B_MSIL2A_20170723T155909_R097_T17SPV_20210210T132957,0.01587150505651398,1405.0,29,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/17/S/PV/2017/07/23/S2B_MSIL2A_20170723T155909_N0212_R097_T17SPV_20210210T132957.SAFE/GRANULE/L2A_T17SPV_A001982_20170723T161238/IMG_DATA/R10m/T17SPV_20170723T155909_TCI_10m.tif
ef04be46891bd8bf9a9beab01aa74b4f,S2A_MSIL2A_20200606T153911_R011_T18TXL_20200826T085139,0.01453896490184414,8456.0,23,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/18/T/XL/2020/06/06/S2A_MSIL2A_20200606T153911_N0212_R011_T18TXL_20200826T085139.SAFE/GRANULE/L2A_T18TXL_A025891_20200606T155142/IMG_DATA/R10m/T18TXL_20200606T153911_TCI_10m.tif
1969e5e476b5971a377c268c7a8a9ca3,S2B_MSIL2A_20200708T184919_R113_T10SEH_20200912T124306,0.014730029943011688,1853.0,3,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/10/S/EH/2020/07/08/S2B_MSIL2A_20200708T184919_N0212_R113_T10SEH_20200912T124306.SAFE/GRANULE/L2A_T10SEH_A017442_20200708T185408/IMG_DATA/R10m/T10SEH_20200708T184919_TCI_10m.tif
3a2c48812b551d720f8d56772efa6df1,S2A_MSIL2A_20190804T154911_R054_T18TVL_20201004T201836,0.03421013743165361,3918.0,24,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/18/T/VL/2019/08/04/S2A_MSIL2A_20190804T154911_N0212_R054_T18TVL_20201004T201836.SAFE/GRANULE/L2A_T18TVL_A021501_20190804T160118/IMG_DATA/R10m/T18TVL_20190804T154911_TCI_10m.tif
6696747608f3d2f469b3e3c28ef9866d,S2A_MSIL2A_20200712T155911_R097_T17SPV_20200912T223337,0.030148720999405115,2835.0,13,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/17/S/PV/2020/07/12/S2A_MSIL2A_20200712T155911_N0212_R097_T17SPV_20200912T223337.SAFE/GRANULE/L2A_T17SPV_A026406_20200712T161123/IMG_DATA/R10m/T17SPV_20200712T155911_TCI_10m.tif
9c601f226c2af07d570134127a7fda27,S2B_MSIL2A_20170723T155909_R097_T17SPV_20210210T132957,0.01587150505651398,1513.0,29,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/17/S/PV/2017/07/23/S2B_MSIL2A_20170723T155909_N0212_R097_T17SPV_20210210T132957.SAFE/GRANULE/L2A_T17SPV_A001982_20170723T161238/IMG_DATA/R10m/T17SPV_20170723T155909_TCI_10m.tif
ef04be46891bd8bf9a9beab01aa74b4f,S2A_MSIL2A_20200616T153911_R011_T18TXL_20210515T065520,0.001570493753718025,8529.0,13,https://sentinel2l2a01.blob.core.windows.net/sentinel2-l2/18/T/XL/2020/06/16/S2A_MSIL2A_20200616T153911_N0212_R011_T18TXL_20210515T065520.SAFE/GRANULE/L2A_T18TXL_A026034_20200616T153934/IMG_DATA/R10m/T18TXL_20200616T153911_TCI_10m.tif
16 changes: 13 additions & 3 deletions tests/test_experiment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from pydantic import ValidationError
import pytest
from typer.testing import CliRunner
import yaml

from cyfi.config import FeaturesConfig
from cyfi.experiment import app, ExperimentConfig
Expand All @@ -28,15 +29,24 @@ def test_experiment_config(train_data_path):
)


def test_cli_experiment(experiment_config_path):
def test_cli_experiment(experiment_config_path, tmp_path):
# use tmp_path as the save location
with experiment_config_path.open("r") as f:
config = yaml.safe_load(f)

config["save_dir"] = str(tmp_path)
new_config_path = tmp_path / "config_artifact.yaml"
with new_config_path.open("w") as fp:
yaml.dump(config, fp)

# Run CLI command
result = runner.invoke(
app,
[str(experiment_config_path)],
[str(new_config_path)],
)
assert result.exit_code == 0

config = ExperimentConfig.from_file(experiment_config_path)
config = ExperimentConfig.from_file(new_config_path)

# Check that artifact config, model zip, and predictions got saved out
for file in ["config_artifact.yaml", "model.zip", "preds.csv"]:
Expand Down
2 changes: 1 addition & 1 deletion tests/test_features.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ def test_generate_candidate_metadata(train_data, features_config):

# Check that candidate metadata matches known expected values
assert candidate_meta.item_id.is_unique
assert len(candidate_meta) == 31
assert len(candidate_meta) == 34
assert (
"S2A_MSIL2A_20170728T155901_R097_T17SPV_20210210T154351" in candidate_meta.item_id.values
)
Expand Down