Retrieve, plot, and sample from Metaculus community prediction distributions #61

brachbach · 2020-04-16T06:18:32Z

These features are used in notebooks/community_distributions_v2.ipynb. You should probably start there to understand what's going on.

Main changes:

get the community prediction from Metaculus
graph the community prediction and our prediction
test this functionality
various small improvements to make it work, e.g. improve how we convert samples to a dataframe

semi-related changes:

change how log questions are graphed
fix some prediction submission stuff with the Metaculus API
fix sample normalization per correspondence with Metaculus

Unrelated changes:

add some development tips to the README (sorry, should really have been a separate PR!)

Some things are still a bit rough:

But I think it's probably better to go ahead and merge this and then fix those

(note that I've assigned the last 3 to myself to work on soon)

…g some names missing from some rows

…d values. Getting some errors.

…on for the log test question is about exactly 1 when it should be around 30k

…he proper domain, but seems weird that normalize's domain can't take things to the left of the question interval

…es much better at plotting the logs of the values directly, for whatever reason. So I should figure out how to do that instead.

brachbach · 2020-04-16T06:20:11Z

ergo/metaculus.py

-        samples = np.maximum(samples, epsilon)
-        samples = samples / self.question_range["min"]
-        return np.log(samples) / np.log(self.deriv_ratio)
+    def normalized_from_true_value(self, true_value) -> float:


These changes are per advice from Max (Metaculus programmer)

pullrequest

✅ A review job has been created and sent to the PullRequest network.

Check the status or cancel PullRequest code review here - or - cancel by adding [!pr] to the title of the pull request.

pullrequest

	1 Message
⚠️	Due to its size, this pull request will likely have a little longer turnaround time and will probably require multiple passes from our reviewers.

pullrequest

I think the general architecture of things is looking pretty good. Although, there are some things, which could be cleaned up a little bit.

From a high-level, it may make sense to have more of the distribution calculations in torch to avoid converting back and forth as much, but that is manage-able.

Gershon B

Reviewed with ❤️ by PullRequest

pullrequest · 2020-04-16T16:19:28Z

ergo/metaculus.py

+        sample_above_range = abs(np.random.logistic(
+            1, 0.02))
+        sample_in_range = ppl.sample(self.community_dist_in_range(
+        )) / float(len(self.prediction_histogram))


Seems like there is a missing argument here. Either way, it would help to add indentation like you have for the other sample ranges.

also just to be sure, is self.prediction_histogram always non-empty? It may be worth to check the denominator and set a default value if it's 0.

Brooks C

(note that one way to avoid discussion around formatting would be to use black for this project:

https://black.readthedocs.io/en/stable/ )

Olivier LF

Thanks, I like the idea of switching to Black and created an issue for it: https://github.com/oughtinc/ergo/issues/80

no missing argument I don't think -- note that we're using a different function to grab a sample than for the other 2

yeah I think self.prediction_histogram is always non-empty -- would probably rather that we just get an error if it's not so that we know that something unexpected happened (rather than handling that case)

pullrequest · 2020-04-16T16:19:29Z

ergo/metaculus.py

+
+    def sample_normalized_community(self):
+        sample_below_range = -1 * \
+            abs(np.random.logistic(0, 0.02))


Also, I think it would be helpful to add a comment on why 0.02 is specified as the scale for maintainability purposes. especially if it's also used for sample_above_range

Brooks C

pullrequest · 2020-04-16T16:19:29Z

ergo/metaculus.py

-        columns = ["id", "name", "title", "resolve_time"]
-        data = []
+        show_names = False
        for question in questions:


For more pythonic-refactor, you could try using the any() builtin function instead. It takes in an iterable and as long as one item in the iterable is True, it will return true. Also if the iterable is empty, it will return False.

It would simplify lines 105-109 to this:

show_names = any(q.name for q in questions)

Brooks C

thanks for not just saying, "wtf is this, just use any!" :p

pullrequest · 2020-04-16T16:19:29Z

ergo/ppl.py

    model = name_count(model)
-    samples: Dict[str, List[float]] = {}
+    samples: List[Dict[str, float]] = []
    for i in tqdm.trange(num_samples):


ISSUE: W0612 (Severity: Low)
Unused variable 'i'

Remediation:
We could probably just do for _ in tqdm.trange(num_samples): if we aren't using i.

🤖 powered by PullRequest Automation 👋 verified by Gershon B

pullrequest · 2020-04-16T16:19:29Z

ergo/metaculus.py

+
+        return ax
+
+    def show_submission(self, samples, show_community=False):


ISSUE: W0221 (Severity: Low)
Parameters differ from overridden 'show_submission' method

Remediation:
show_community should be in abstract class if used here.

🤖 powered by PullRequest Automation 👋 verified by Gershon B

yeah I left this as a loose end. See https://github.com/oughtinc/ergo/issues/64

pullrequest · 2020-04-16T16:19:31Z

ergo/metaculus.py

+        # {'prediction': ['high minus low must be at least 0.01']}"
        high = max(min(distribution.cdf(1), 0.99),
-                   0.0099) if self.high_open else 1
+                   low + 0.01) if self.high_open else 1


I'd recommend creating a descriptive constant for 0.01 instead of hard-coding it.

Gershon B

pullrequest · 2020-04-16T16:19:31Z

ergo/metaculus.py

+
+    def sample_normalized_community(self):
+        sample_below_range = -1 * \
+            abs(np.random.logistic(0, 0.02))


It may also be good have a constant for 0.02.

Gershon B

pullrequest · 2020-04-16T16:19:31Z

ergo/metaculus.py

+    def denormalize_samples(self, samples):
+        raise NotImplementedError("This should be implemented by a subclass")
+
+    @functools.lru_cache(None)


I'd caution against using functools.lru_cache for class methods[1]. Is prediction_histogram only set once? Would @functools.cached_property be a better choice?

[1] https://stackoverflow.com/questions/33672412/python-functools-lru-cache-with-class-methods-release-object

Gershon B

(note that functools.cached_property requires python 3.8)

Olivier LF

The problem is that the cache will hold onto the class instance in memory?

I don't see that causing performance problems soon, so I'm probably inclined to just leave it in

pullrequest · 2020-04-16T16:19:31Z

ergo/metaculus.py


        self.show_prediction(submission, samples)

+    def show_community_prediction(self, only_show_this=True):


ISSUE: W0221 (Severity: Low)
Parameters differ from overridden 'show_community_prediction' method

Remediation:
only_show_this should probably in the base class if we are going to use it here.

🤖 powered by PullRequest Automation 👋 verified by Gershon B

yeah I left this as a loose end. See #64

pullrequest · 2020-04-16T16:19:32Z

ergo/metaculus.py

+    @functools.lru_cache(None)
+    def community_dist_in_range(self):
+        # distribution on integers referencing 0...(len(self.prediction_histogram)-1)
+        df = pd.DataFrame(self.prediction_histogram, columns=["x", "y1", "y2"])


It seems like we don't need to create a DataFrame if we are just going to create a torch Tensor. Perhaps,

y2 = [p[2] for p in self.prediction_histogram] return dist.Categorical(probs=torch.Tensor(y2))

or

t = torch.Tensor(self.prediction_histogram) return dist.Categorical(probs=t[:,2])

would work.

Gershon B

pullrequest

This looks neat! Added some style comments and perhaps the graphs in the notebook can be made easier to read at a glance with more human readable units for number of occurrences on the x-axis

Olivier LF

Reviewed with ❤️ by PullRequest

pullrequest · 2020-04-16T17:51:15Z

ergo/metaculus.py

+
+    @functools.lru_cache(None)
+    def community_dist_in_range(self):
+        # distribution on integers referencing 0...(len(self.prediction_histogram)-1)


Converting this to a docstring will allow automated doc generation frameworks or code introspection to more easily extract this information

"""Return the distribution on integers referencing 0...(len(self.prediction_histogram)-1)"""

Olivier LF

pullrequest · 2020-04-16T17:51:16Z

ergo/metaculus.py

 import ergo.logistic as logistic
+import ergo.ppl as ppl

 from typing import Optional, List, Any, Dict, Tuple


ISSUE: W0611 (Severity: Low)
Unused Tuple imported from typing

Remediation:
Nitpick: similarly could also consider removing the unused Tuple

🤖 powered by PullRequest Automation 👋 verified by Olivier LF

pullrequest · 2020-04-16T17:51:16Z

ergo/metaculus.py

+import torch
 import requests
 import pendulum
 import scipy


ISSUE: W0611 (Severity: Low)
Unused import scipy

Remediation:
Won't make much of a difference but could perhaps slightly speed things up by removing the unused import.

🤖 powered by PullRequest Automation 👋 verified by Olivier LF

pullrequest · 2020-04-16T17:51:16Z

notebooks/community_distributions_v2.ipynb

+        "\n",
+        "print([log_tick for log_tick in pyplot.xticks()])\n",
+        "\n",
+        "pyplot.xticks(pyplot.xticks()[0], [f\"{math.exp(log_tick):.1e}\" for log_tick in pyplot.xticks()[0]], rotation=\"vertical\")\n",


Perhaps a LogFormatter could make the x ticks more legible here?

https://stackoverflow.com/a/43926354/163677

Olivier LF

pullrequest · 2020-04-16T17:51:16Z

ergo/metaculus.py

        latest_prediction = self.my_predictions["predictions"][-1]["d"]
        return self.get_submission_from_json(latest_prediction)

+    def show_community_prediction(self):


Adding a docstring explaining what subclasses will have to implement, and what behavior should be expected will be rather useful to ease extensibility by users less familiar with the codebase.

Olivier LF

I moved all of the stubs to the bottoms of their respective classes

I suspect that doing more than that will be more trouble than it's worth

pullrequest · 2020-04-16T17:51:16Z

ergo/metaculus.py

+        return dist.Categorical(probs=torch.Tensor(df["y2"]))
+
+    def sample_normalized_community(self):
+        sample_below_range = -1 * \


sample_below_range = -abs(np.random.logistic(0, 0.02))

might be clearer? It seems to work as expected:

>>> -abs(-4) -4

Olivier LF

brachbach · 2020-04-17T02:14:46Z

@stuhlmueller -- I made the vast majority of the changes requested by PullRequest, and commented on most others about why I didn't make the change.

OK to merge this once CI passes?

codecov · 2020-04-17T02:32:17Z

Codecov Report

Merging #61 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #61   +/-   ##
=======================================
  Coverage   76.08%   76.08%           
=======================================
  Files           7        7           
  Lines         623      623           
=======================================
  Hits          474      474           
  Misses        149      149

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6aedad9...6aedad9. Read the comment docs.

stuhlmueller and others added 27 commits April 3, 2020 21:20

WIP sampling from community predictions

834972f

Add WIP community distributions notebook

413d376

Move community distributions notebook

69245b6

Update README.md

e5a6d9d

Merge branch 'master' into community-prediction

db43210

Merge in submit_mixture, tests pass

98e66a1

Merge branch 'submit_mixture' into community-prediction

4337c2a

Add local runtime instructions

6719335

Add PR submission instructions

533342e

Fix bug introduced when merging in other branches

b22f9ea

Remove ill-advised defensive programming

38f4054

Try to show prediction but it's on the wrong plot

ae47c19

Add df rows as individual dicts so that there's no problem with havin…

935d65c

…g some names missing from some rows

Remove extraneous file

16762d4

Plot community and our prediction on the same graph

b042432

Merge branch 'master' into community-prediction

0d85580

Use formula from Max for converting between true values and normalize…

be17f04

…d values. Getting some errors.

Fixed some infra issues and improved code, but now my median predicti…

ed7f6db

…on for the log test question is about exactly 1 when it should be around 30k

This formula doesn't make sense, must be wrong

f1ed1e1

Fix formulae so that they're actually inverses of each other within t…

0723a8c

…he proper domain, but seems weird that normalize's domain can't take things to the left of the question interval

These formulae seem to be the correct interpretations

e8ec58c

Use more sensible flooring for normalizing log predictions

2a7dc4d

Get really close to reconstructing the community predictions

08d88a3

Fix comment

b835305

Improve plots somewhat by tweaking them. But I notice that seaborn do…

8728e91

…es much better at plotting the logs of the values directly, for whatever reason. So I should figure out how to do that instead.

Get nice graphs for log predictions. Code is a little messy though.

d319988

Merge in master

fa7114f

brachbach commented Apr 16, 2020

View reviewed changes

pullrequest bot reviewed Apr 16, 2020

View reviewed changes

brachbach requested review from stuhlmueller and william-r-s April 16, 2020 06:34

Add new version of community distributions notebook and link in README

02433d2

pullrequest bot reviewed Apr 16, 2020

View reviewed changes

stuhlmueller assigned brachbach Apr 16, 2020

stuhlmueller changed the title ~~Community prediction~~ Retrieve, plot, and sample from Metaculus community prediction distributions Apr 16, 2020

brachbach added 2 commits April 16, 2020 19:07

Implement suggestions from PullRequest

67dea72

Merge in master

461c1c2

Manually fix flake formatting errors

6aedad9

stuhlmueller merged commit 4156448 into master Apr 17, 2020

brachbach deleted the community-prediction branch April 28, 2020 03:56


		return ax

		def show_submission(self, samples, show_community=False):


		self.show_prediction(submission, samples)

		def show_community_prediction(self, only_show_this=True):

Retrieve, plot, and sample from Metaculus community prediction distributions #61

Retrieve, plot, and sample from Metaculus community prediction distributions #61

Uh oh!

Conversation

brachbach commented Apr 16, 2020 • edited by stuhlmueller Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pullrequest bot left a comment

Choose a reason for hiding this comment

Uh oh!

pullrequest bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pullrequest bot left a comment

Choose a reason for hiding this comment

Uh oh!

pullrequest bot Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

pullrequest bot Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pullrequest bot Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

pullrequest bot Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pullrequest bot Apr 16, 2020

Choose a reason for hiding this comment

🤖 powered by PullRequest Automation 👋 verified by Gershon B

Uh oh!

pullrequest bot Apr 16, 2020

Choose a reason for hiding this comment

🤖 powered by PullRequest Automation 👋 verified by Gershon B

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pullrequest bot Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

pullrequest bot Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

pullrequest bot Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

pullrequest bot Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pullrequest bot Apr 16, 2020

Choose a reason for hiding this comment

🤖 powered by PullRequest Automation 👋 verified by Gershon B

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pullrequest bot Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

pullrequest bot left a comment

Choose a reason for hiding this comment

Uh oh!

pullrequest bot Apr 16, 2020

Choose a reason for hiding this comment

Uh oh!

pullrequest bot Apr 16, 2020

Choose a reason for hiding this comment

brachbach commented Apr 16, 2020 •

edited by stuhlmueller

Loading

pullrequest bot left a comment •

edited

Loading