Skip to content

Add reference data for sentinel1_sar_slc_preprocessing test.#482

Merged
EmileSonneveld merged 1 commit intomainfrom
sentinel1_sar_slc_preprocessing_reference_data
Apr 27, 2026
Merged

Add reference data for sentinel1_sar_slc_preprocessing test.#482
EmileSonneveld merged 1 commit intomainfrom
sentinel1_sar_slc_preprocessing_reference_data

Conversation

@EmileSonneveld
Copy link
Copy Markdown
Collaborator

@EmileSonneveld EmileSonneveld commented Apr 27, 2026

No description provided.

@EmileSonneveld
Copy link
Copy Markdown
Collaborator Author

@copilot review without making code changes

@algorithm-services-catalogue
Copy link
Copy Markdown

algorithm-services-catalogue Bot commented Apr 27, 2026

🧹 Catalogue's Preview Site Cleanup

The preview site for this pull request has been cleaned up.

If you need a new preview, please reopen the PR or create a new one.

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 27, 2026

@copilot review without making code changes

Here's my review of the changes (commit 6e474f9):

Change summary: Adds 9 entries to reference_data in sentinel1_sar_slc_preprocessing.json — 8 .tif files and a job-results.json — all pointing to run #541 on s3.waw3-1.cloudferro.com/apex-benchmarks-internal.

Observations / potential issues:

  1. tif_0 files look suspicious: The two entries S1_2images_prm_20180128T062713_vh_grid_lat.tif_0 and S1_2images_prm_20180128T062713_vh_grid_lon.tif_0 are unlike any other reference data files in the repo. No other benchmark scenario uses this _0 suffix naming pattern. This could be a GDAL overview/internal file artifact that was incidentally uploaded alongside the real .tif files, rather than a genuine expected output. If the actual job run doesn't produce these _0 files, the benchmark comparison will fail.

  2. More files than comparable scenarios: Other eurac scenarios (sentinel1_sar_coherence, sentinel1_sar_interferogram) reference only 2 files (job-results.json + 1 .tif). This scenario references 9, which may be correct given SLC preprocessing produces multiple output bands/grids, but worth confirming that all 8 .tif files (including the _0 ones) are genuinely expected outputs.

  3. Pinned namespace SHA: The namespace URL uses a pinned commit SHA (2ed1e3dc…), which is good for reproducibility, but it's pinned to an older commit. Depending on the project's conventions, refs/heads/main or a more recent pinned SHA might be preferred.

  4. Schema validation: ✅ Passes benchmark_scenario.json schema validation.

  5. URL format: All URLs follow the same pattern as other benchmark scenarios and are well-formed.

Verdict: The main concern is whether the tif_0 files are real expected outputs or artifacts from the upload process. If they're artifacts, they should be removed to avoid spurious test failures.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • s3.waw3-1.cloudferro.com
    • Triggering command: `/usr/bin/python3 python3 -c
      import requests

Test a few of the URLs to see if they are accessible

urls = [
'REDACTED!tests_test_benchmarks.py__test_run_benchmark_sentinel1_sar_slc_preprocessing_!actual/S1_2images_prm_201801` (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@EmileSonneveld
Copy link
Copy Markdown
Collaborator Author

S1_2images_prm_20180128T062713_vh_grid_lat.tif_0
and
S1_2images_prm_20180128T062713_vh_grid_lat.tif
refer to the same tiff. The same file get referred twice in this catalog, probably being the cause of this name conflict. It gets resolved nicely tough

@EmileSonneveld EmileSonneveld merged commit 798906d into main Apr 27, 2026
5 checks passed
@EmileSonneveld EmileSonneveld deleted the sentinel1_sar_slc_preprocessing_reference_data branch April 27, 2026 08:38
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