Skip to content

Add individual correction metadata to CSLC-S1 #156

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

Merged
merged 5 commits into from
May 26, 2023

Conversation

vbrancat
Copy link
Contributor

This PR adds some metadata in CSLC-S1 product to indicate which correction has been applied.
Also corrects the snake VS camel case inconsistency in the name of the flattening flags.

Can you check if the description of the individual correction metadata looks clear?

Copy link
Contributor

@LiangJYu LiangJYu left a comment

Choose a reason for hiding this comment

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

LGTM

@vbrancat vbrancat requested a review from seongsujeong May 26, 2023 03:39
@vbrancat
Copy link
Contributor Author

@seongsujeong this PR is ready to be reviewed

Copy link
Contributor

@seongsujeong seongsujeong left a comment

Choose a reason for hiding this comment

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

LGTM overall. Here I left couple of questions that you might want to check.
Approving this PR in advance expedite the process because my comments are minor
-- Seongsu

Comment on lines +586 to +587
Meta('los_solid_earth_tides_applied', bool(cfg.lut_params.enabled),
"If True, solid Earth tides correction has been applied in slant range direction"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering if we will need to put a placeholder for sth. like azimuth_solid_earth_tides_applied

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not a bad idea. Just in case it will be more difficult to change the product

Comment on lines +588 to +589
Meta('static_troposphere_applied', bool(cfg.lut_params.enabled),
"If True, troposphere correction based on a static model has been applied"),
Copy link
Contributor

Choose a reason for hiding this comment

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

What is going to happen for static troposphere correction if cfg.lut_params.enabled is True, and we apply weather-model based troposphere correction? I think this field has to be False in that case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My hope is that when we include the individual enabling/disabling of the LUTs, each of these flags will be replaced by its own individual flag. So later on, we should have the ability to set to False (or True) each of these corrections

@vbrancat vbrancat merged commit 64ffe40 into opera-adt:main May 26, 2023
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