-
Notifications
You must be signed in to change notification settings - Fork 5
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
Updated exposure function call: enable temporal filtering #88
Conversation
… call in notebook
Removed a outstanding references to spatial filtering and ensured script is up to date and consistent with closed PR #85
For full integration test results, refer to the Tests directory README. |
intertidal/elevation.py
Outdated
) | ||
|
||
# Write each exposure output as new variables in the main dataset | ||
for x in exposure_filters.data_vars: |
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 think given we're only ever going to be extracting the "unfiltered" layer here, I think we can simplify this code to:
ds["exposure"] = exposure_filters["unfiltered"]
# Calculate the tide-height difference between the elevation value and | ||
# each percentile value per pixel | ||
diff = abs(tide_cq - dem) | ||
modelledtides_1d = modelledtides_lowres.mean(dim=["x", "y"]) |
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.
There's a similar code block below this that is commented out - can you have a look and see if we can remove one of these?
intertidal/elevation.py
Outdated
# Calculate exposure | ||
ds["exposure"], tide_cq = exposure( | ||
exposure_filters, tide_cq_dict = exposure( |
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 the function below, tide_cq_dict
is called modelledtides_ds
- can we use that terminology here too for consistency?
(are these raw modelled tides or tide quantiles? Alternatively, we could call it something like tide_quantiles_ds
too if that was more clear)
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'll update that variable name. It's a legacy hangover from an earlier version of the code.
Co-authored-by: Robbi Bishop-Taylor <[email protected]>
Co-authored-by: Robbi Bishop-Taylor <[email protected]>
Co-authored-by: Robbi Bishop-Taylor <[email protected]>
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.
Wooo!
This PR updates the exposure function to incorporate the ability to include temporal filtering in the exposure output/s. Incorporating this update into the intertidal repo is required to enable moving the exposure demo notebook into the dea_notebooks repository whilst ensuring enabled functionality is provided by the intertidal repository.
Changes and updates:
None
so theoretically, no updates should be required to the remainder of our production code base.Intertidal_workflow.ipynb
.Future work: