-
Notifications
You must be signed in to change notification settings - Fork 32
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
Weighted Average #833
base: main
Are you sure you want to change the base?
Weighted Average #833
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
I've set up the boilerplate for the weighted mean functionality. This should be a good place to get started. We can run over this during today's meeting. |
We have fixed the issue with the |
ASV BenchmarkingBenchmark Comparison ResultsBenchmarks that have improved:
Benchmarks that have stayed the same:
Benchmarks that have got worse:
|
…rray/weighted-mean (#866) * updated mean function with weighted arg * updated weighted-mean functionality in dataarray.py * edited weights to dask array --------- Co-authored-by: Rachel Yuen Sum Tam <[email protected]> Co-authored-by: Rachel Yuen Sum Tam <[email protected]>
nt.assert_equal(result, expected_weighted_mean) | ||
|
||
|
||
def test_csne30_equal_area(): |
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.
weighted_mean = (self * weights).sum(axis=-1) / total_weight | ||
|
||
# create a UxDataArray and return it | ||
return UxDataArray(weighted_mean, uxgrid=self.uxgrid) |
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.
Need to preserve other parameters:
- Coordiantes
UXDataset support |
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! Just a few suggestions.
# compute the total weight | ||
total_weight = weights.sum() | ||
|
||
# compute weighted mean #assumption on index of dimension (last one is geometry) |
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.
# compute weighted mean #assumption on index of dimension (last one is geometry) | |
# compute the weighted mean, with an assumption on the index of dimension (last one is geometry) |
This function calculates the weighted mean of a variable, | ||
using the specified `weights`. If no weights are provided, it will automatically select | ||
appropriate weights based on whether the variable is face-centered or edge-centered. If | ||
the variable is neither face nor edge-centered. |
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 variable is neither face nor edge-centered. | |
the variable is neither face nor edge-centered a warning is raised, and an unweighted mean is computed instead. |
uxds = ux.open_dataset(quad_hex_grid_path, quad_hex_data_path_face_centered) | ||
|
||
# expected weighted average computed by hand | ||
expected_weighted_mean = 297.55 |
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.
Can you compute this within Python? I think if you can avoid hard coding in the answer that is ideal, although I know this can't always be avoided.
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.
+1 this! Even if it is inevitable to use constants, they shouldn't be showing up as magic numbers within the code and instead should go into some kind of test constants
Closes #826
Overview
UxDataArray