Eqw 2 #94
Merged
Eqw 2 #94
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 9 commits
April 29, 2020 00:35
znicholls
approved these changes
Apr 30, 2020
| Database cruncher which uses the 'equal quantile walk' technique. | ||
|
|
||
| This cruncher assumes that the amount of effort going into reducing one emission set | ||
| is equal to that for another emission, therefore the lead and follow data should be |
There was a problem hiding this comment.
Suggested change
| is equal to that for another emission, therefore the lead and follow data should be | |
| is equal to that for another emission, therefore the lead and follow data should come from |
| This cruncher assumes that the amount of effort going into reducing one emission set | ||
| is equal to that for another emission, therefore the lead and follow data should be | ||
| the same quantile of all pathways in the infiller database. | ||
| It calculates what quantile the lead infillee data is in the lead infiller database, |
There was a problem hiding this comment.
Suggested change
| It calculates what quantile the lead infillee data is in the lead infiller database, | |
| It calculates the quantile of the lead infillee data is in the lead infiller database, |
| lead_vals = lead_vals.sort_values() | ||
| quant_of_lead_vals = np.arange(len(lead_vals)) / (len(lead_vals) - 1) | ||
| if any(quant_of_lead_vals > 1) or any(quant_of_lead_vals < 0): | ||
| raise NotImplementedError("Impossible quantiles!") |
There was a problem hiding this comment.
Suggested change
| raise NotImplementedError("Impossible quantiles!") | |
| raise ValueError("Impossible quantiles!") |
Also very amusing +1
| input_quantiles = scipy.interpolate.interp1d( | ||
| lead_vals, quant_of_lead_vals, bounds_error=False, fill_value=(0, 1) | ||
| )(lead_input) | ||
| return np.nanquantile(follow_vals, input_quantiles) |
There was a problem hiding this comment.
Suggested change
| return np.nanquantile(follow_vals, input_quantiles) | |
| return np.nanquantile(follow_vals, input_quantiles, interpolation="linear") |
It's already what happens but just makes clear?
| return self._db.filter(variable=variable_follower) | ||
|
|
||
| def _find_same_quantile(self, follow_vals, lead_vals, lead_input): | ||
| if len(lead_vals) == 1: |
There was a problem hiding this comment.
Suggested change
| if len(lead_vals) == 1: | |
| if len(follow_vals) == 1: |
Would this be clearer?
There was a problem hiding this comment.
It's to avoid a singularity later, we could also short-circuit the calculation in the case of 1 follow but that's for computational reasons not essential ones. I guess it's better to take the mean afterwards in case the length of the two are different.
|
|
||
| infilled = res(simple_df) | ||
| # We compare the results with the expected results: for T1, we are below the | ||
| # lower limit on the first, in the middle on the second. At later times we are |
There was a problem hiding this comment.
Suggested change
| # lower limit on the first, in the middle on the second. At later times we are | |
| # lower limit on the first, in the middle on the second scenario. At later times we are |
? Is there a way to make it slightly clearer in the test rather than having the hard-coded 50 and 100?
There was a problem hiding this comment.
It's derived better now
|
|
||
| def test_with_one_value_in_infiller_db(self, test_db, caplog): | ||
| # The calculation is different with only one entry in the infiller db. We | ||
| # expect a warning and the only value to be returned in all cases. |
There was a problem hiding this comment.
Suggested change
| # expect a warning and the only value to be returned in all cases. | |
| # |
No warning at the moment ?
There was a problem hiding this comment.
True. Probably not going to add one now, they are annoying
| ) | ||
|
|
||
| # Repeat with reducing the minimum value. This works differently because the | ||
| # minimum point is doubled. By default the cruncher selects the higher |
There was a problem hiding this comment.
Suggested change
| # minimum point is doubled. By default the cruncher selects the higher | |
| # minimum point is doubled. This modification causes the cruncher to pick the lower value. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request
Please confirm that this pull request has done the following:
CHANGELOG.rstaddedAdding to CHANGELOG.rst
Please add a single line in the changelog notes similar to one of the following: