-
Notifications
You must be signed in to change notification settings - Fork 18
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
Weather model-based troposphere delay LUTs #103
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update current docker/specifile.txt with the attached one.
specifile.txt
@LiangJYu not sure why the CI is failing. It mentions a bug in conda. |
only forge - no default or .condarc channels
Fix add raider tropo docker
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this 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. Just a question and a reminder.
docker/Dockerfile
Outdated
@@ -30,7 +30,7 @@ RUN ${CONDA_PREFIX}/bin/conda init bash | |||
COPY --chown=compass_user:compass_user . /home/compass_user/OPERA/COMPASS | |||
|
|||
# create CONDA environment | |||
RUN conda create --name "COMPASS" --file /home/compass_user/OPERA/COMPASS/docker/specifile.txt | |||
RUN conda create --name "COMPASS" --file /home/compass_user/OPERA/COMPASS/docker/specifile.txt -c conda-forge --override-channels |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A question: It seems all dependencies in specifile.txt
has conda-forge
in their URLs (correct me if I missed something). Are those options are really necessary, or just to make sure that all packages are from conda forge?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deferring this question to @LiangJYu who provided the specifile.txt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch @seongsujeong. You're correct what I added here is unnecessary. I got the specifile.txt confused with requirements.txt.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LiangJYu will you take care of this?
f'Wet LOS troposphere delay {desc}', | ||
{'units': 'meters'})) | ||
if 'dry' in delay_type: | ||
correction_items.append(Meta('dry_los_troposphere_delay', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does not need to be addressed in this PR, but reminding that we will need to rearrange the order of the corrections to write into the output HDF5 file because we have SET azimuth, and ionosphere not in place yet.
I'll submit a WIP PR to azimuth SET conversion, and let's utilize that PR to finalize all the corrections we have implemented so far.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be we can open an issue to track this down @seongsujeong ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tasks for the finalization will be tracked here:
#106
if weather_model_path is not None: | ||
check_file_path(weather_model_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case the weather model file is not provided, we can still apply troposphere delay using static model in python level because we have all ingredients for the calculation. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. I think we should apply the static troposphere correction regardless. Only, have we decided if we want to proceed with the computation of the static delay at the core module or at a Python level? I am in favor of the second, it should not be that expensive to compute anyway.
Anyhow, I would recommend to add the static troposphere in another PR and merge this one which is needed for the Gamma delivery
* Add RAiDER to the requirements file * Exposed troposphere to the CSLC interface * Compute troposphere delay using weather model * Save computed troposphere range LUT * Add Docker spec file * Format delay_type logic to avoid repetition * reduce line of codes * fix specifile only forge - no default or .condarc channels * correctly force forge w/o defauluts * Correct zenith delay formula * Remove unnecessary flags that forced conda forge channel --------- Co-authored-by: Liang Yu <liangjyu@gmail.com>
This PR adds the computation of the model-based troposphere delay computation to the generation of a CSLC-S1 product.
The wet and dry troposphere delays are computed using the RAiDER software. Note: