Skip to content

Conversation

@abradley60
Copy link
Collaborator

@abradley60 abradley60 commented May 16, 2025

  • Incorperate REMA in the sar-pipeline

@abradley60 abradley60 marked this pull request as ready for review May 19, 2025 04:13
@abradley60 abradley60 requested review from caitlinadams and geoscience-aman and removed request for geoscience-aman May 19, 2025 04:13
Copy link
Collaborator

@geoscience-aman geoscience-aman left a comment

Choose a reason for hiding this comment

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

Thanks Alex!


```bash
docker run --env-file env.secret -it sar-pipeline --scene S1A_IW_SLC__1SSH_20220101T124744_20220101T124814_041267_04E7A2_1DAD --output_crs 3031 --burst_id_list t070_149815_iw3
docker run --env-file env.secret -it sar-pipeline --scene S1A_IW_SLC__1SSH_20220101T124744_20220101T124814_041267_04E7A2_1DAD --output_crs 3031 --burst_id_list t070_149815_iw3 --skip_upload_to_s3
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just thinking - if you want to test locally like you are here with the --skip_upload_to_s3 flag, wouldn't you want to set make_existing_products = True? To tell it to not look at existing products on S3, because you're not going to upload there anyway.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably a good idea, I'll change to reflect

- `resolution` -> The target resolution of the products. Default is 20m.
- `output_crs` -> The target crs of the products. If not specified, the UTM of the scene center will be used. Expects integer values (e.g. 3031)
- `dem` -> The dem to be used in processing. Supported is `cop_glo30`.
- `dem_type` -> The dem to be used in processing. Supported is [`cop_glo30`, `REMA_32`, `REMA_10`, `REMA_2`].
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggest

The digital elevation model (DEM) to use for processing. Supported values: cop_glo30, REMA_32, REMA_10, REMA_2.

- `s3_project_folder` -> The project folder to upload to.
- `collection` -> The collection which the set of products belongs.
- `make_existing_products` -> If products should be made, even if they already exist in S3.
- **WARNING**, - Setting will create duplicate files and overwrite metadata that will impact downstream processes.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggest

Whether to generate products even if they already exist under the s3_bucket/s3_project_folder/collection.
WARNING - Passing this flag will create duplicate files and overwrite existing metadata, which may affect downstream workflows.

@click.option(
"--dem-type",
required=True,
type=click.Choice(["cop_glo30", "REMA_32", "REMA_10", "REMA_2"]),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be worth adding a help statement here.

Copy link
Collaborator

@caitlinadams caitlinadams left a comment

Choose a reason for hiding this comment

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

Looks good to me -- one minor comment suggesting adding a help statement for the CLI for dem_type

@abradley60 abradley60 merged commit 436a254 into main May 20, 2025
2 checks passed
@abradley60 abradley60 deleted the upgrade/rema-fix branch May 26, 2025 04:59
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.

3 participants