Add unknown T₂, Gaussian hyperparameterized models. #146

cgranade · 2018-08-06T23:14:12Z

This PR adds two new models, one which learns Rabi frequencies with unknown T₂ processes, the other of which decorates two-outcome models with Gaussian hyperparameters based on a latent two-outcome variable. This allows for using QInfer to learn T₂ in cases where single-shot measurements are not directly observed, but are only observed through a secondary process that is Gaussian distributed.

coveralls · 2018-08-06T23:57:48Z

Coverage decreased (-0.6%) to 74.549% when pulling acbd0c9 on cgranade/unknown-T2 into 64d9b2f on master.

ihincks

Good addition.

ihincks · 2018-08-07T15:33:57Z

src/qinfer/derived_models.py

+    ## METHODS ##
+
+    def domain(self, expparams):
+        return [RealDomain()] * len(expparams)


domain() Should support "in the case where n_outcomes_constant is True, None should be a valid input".

This requirement is a side-effect of my poor code design IMO, however, we should stick to it until something smoother comes along.

Ah, good catch, thank you. I'll go on and fix that, then.

ihincks · 2018-08-07T15:36:45Z

src/qinfer/derived_models.py

+        #     (idx_underlying_outcome, idx_outcome, idx_modelparam, idx_experiment).
+        # Thus, we need shape
+        #     (idx_underlying_outcome,           1, idx_modelparam,              1).
+        mu = (modelparams[:, -4:-2].T)[:, None, :, None]


np.newaxis is used a bit more consistently elsewhere, I think?

Good point, I was being a bit sloppy here, but np.newaxis is much easier to read. I'll fix that, then.

ihincks · 2018-08-07T15:41:00Z

src/qinfer/derived_models.py

+        return True
+
+    def are_models_valid(self, modelparams):
+        orig_mps = modelparams[:, :-4]


FYI negative indexes (rather than absolute indexes based on underlying_model.n_modelparams) have occasionally caused me trouble when another decoration model is above. However, I don't think anything in the code base actually cares, and I think other models might use negative indexes too, so don't change it if you don't want to. It mostly comes up when I am manually jiggering things in a notebook, and have many models and many updaters on the go.

It's easy enough to split things out in a more robust way, which would really help for more complicated model chains. Thanks for pointing that out!

ihincks · 2018-08-07T15:42:03Z

src/qinfer/derived_models.py

+            (modelparams[:, -2:].T)[:, None, :, None]
+        )
+
+        assert np.all(sigma > 0)


Is this left-over from prototyping?

Very likely, I should either remove it or make it an actual exception.

ihincks · 2018-08-07T15:46:51Z

src/qinfer/test_models.py

+    :modelparam T2_inv: The decoherence strength :math:`T_2^{-1}`.
+    :scalar-expparam float: The evolution time :math:`t`.
+    """
+    # TODO: add duecredit.cite.


ihincks · 2018-08-07T15:47:09Z

src/qinfer/derived_models.py

+
+        # Now we marginalize and return.
+        L = (underlying_L * conditional_L).sum(axis=0)
+        assert not np.any(np.isnan(L))


Is this left-over from prototyping?

Same answer here, sorry for missing them both.

ihincks · 2018-08-07T15:53:15Z

The 4 CI failures look like they have nothing to do with this PR. Are they due to a new numpy thing?

cgranade · 2018-08-07T20:22:32Z

Thanks for the review, @ihincks! I'll go on and fix those ASAP. As far as the CI failures, I think that's leftover from #135, and is due to NumPy finally making something an error that was a FutureWarning for a long time. I'll investigate in #135, then, so that I can start clearing out the backlog of PRs contingent on CI failures.

ihincks · 2018-08-07T20:46:28Z

Consider throwing a test or two into test_concrete_models.py. Untested:

class TestGaussianHyperparameterizedModel(ConcreteModelTest, DerandomizedTestCase):
    """
    Tests GaussianHyperparameterizedModel with CoinModel as the underlying model
    (underlying model has no expparams).
    """

    def instantiate_model(self):
        return GaussianHyperparameterizedModel(CoinModel())
    def instantiate_prior(self):
        return ProductDistribution(
               BetaDistribution(mean=0.5, var=0.1),
               ...
        )
    def instantiate_expparams(self):
return np.arange(100, 120).astype(self.model.expparams_dtype)

taalexander · 2018-08-08T11:51:15Z

This looks great. I agree with @ihincks it is a good addition. It seems as we move forward experimentally there is an increasing focus on continuous outcome distributions. Once I find more time I want to focus on explicitly supporting continuous outcomes (right now IIRC it is maybe hinted at in the documentation but left to the user to figure out how to support them). This would also include supporting the branch me and Ian have been working on to support continuous outcomes natively include experiment design in smc.py. I will try and clean this up and get tests passing later this week so that I can submit a PR. There will be a decent amount of documentation and testing to get it in working condition. I hope that after @ihincks defends he might be able to help me review this and come up with a plan.

cgranade · 2018-08-08T15:29:10Z

Sounds great, @taalexander! Thanks as well for the typo catch, @ihincks. I think this might be good to go once I can get the CI builds working. Would anyone mind if I merge in #135, since CI seems to work there again? That would mean deprecating 3.3 in favor of Python 2.7 or ≥ 3.4. Since 3.3 is now approximately six years old, I don't think that should be a problem...

ihincks · 2018-08-08T17:16:56Z

src/qinfer/derived_models.py

+        # Next, we sample a bunch of underlying outcomes to figure out
+        # how to rescale everything.
+        underlying_outcomes = self.underlying_model.simulate_experiment(
+            modelparams[:, :-4], expparams


repeat=repeat i think will fix the other shape mismatch bug

ihincks · 2018-08-08T17:19:44Z

src/qinfer/derived_models.py

+        assert np.all(sigma > 0)
+
+        # Now we can rescale the outcomes to be random variates z drawn from N(0, 1).
+        scaled_outcomes = (outcomes - mu) / sigma


outcomes[np.newaxis,:,np.newaxis,np.newaxis] will fix one of the shape bugs?

i think numpy must have changed how it assigns to fancy dtypes at some point

ihincks

Go ahead when ready.

I removed init because it was causing a failure; was unaware of its involvement in dcite mechanics. Thanks for fixing.

cgranade · 2018-08-09T01:34:07Z

Awesome, thanks for everything! 💕

Christopher Granade and others added 10 commits April 26, 2018 14:51

New model that hyperparameterizes two-outcome models w/ Gaussians.

a4bef96

Adds an unknown T2 model.

ae1e7fd

Added UnknownT2Model to __all__.

b8864e7

Minor fixes to I/Q model.

e887cd9

Whitespace fixes.

3719a07

Small bugfixes, now works for T₂⁻¹ I/Q learning.

14a1370

Docstring for unknown T₂ model.

83d6bb9

Added unknown T₂ model to docs.

252ceec

Docs for Gaussian hyperparameterized model.

84eada5

Whitespace

ead7ecf

ihincks reviewed Aug 7, 2018

View reviewed changes

cgranade added 3 commits August 7, 2018 18:29

Began addressing @ihincks' feedback.

26656f0

Added missing citation.

d58693a

Test case from @ihincks.

384bb84

fixed dcite typo

b0af2b3

cgranade added 2 commits August 8, 2018 08:38

Merge remote-tracking branch 'origin/master' into cgranade/unknown-T2

e9eff22

Added missing modelparam for CoinModel in new test.

ef46872

ihincks reviewed Aug 8, 2018

View reviewed changes

ihincks and others added 5 commits August 8, 2018 15:32

allow SimplePrecessionModel to accept overcomplete expparams

5094e77

i think numpy must have changed how it assigns to fancy dtypes at some point

fixed some minor indexing issues

4b93447

allow super to set attributes

b79b218

add tests for UnknownT2Model

c2606c0

Refactored out dcite call to avoid defining __init__.

acbd0c9

ihincks approved these changes Aug 8, 2018

View reviewed changes

cgranade merged commit 3c9cc7e into master Aug 9, 2018

cgranade deleted the cgranade/unknown-T2 branch August 9, 2018 01:34

Add unknown T₂, Gaussian hyperparameterized models. #146

Add unknown T₂, Gaussian hyperparameterized models. #146

Uh oh!

Conversation

cgranade commented Aug 6, 2018

Uh oh!

coveralls commented Aug 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ihincks left a comment

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!

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!

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!

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!

ihincks commented Aug 7, 2018

Uh oh!

cgranade commented Aug 7, 2018

Uh oh!

ihincks commented Aug 7, 2018

Uh oh!

taalexander commented Aug 8, 2018

Uh oh!

cgranade commented Aug 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ihincks Aug 8, 2018 • 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!

ihincks left a comment

Choose a reason for hiding this comment

Uh oh!

cgranade commented Aug 9, 2018

Uh oh!

Uh oh!

coveralls commented Aug 6, 2018 •

edited

Loading

ihincks Aug 8, 2018 •

edited

Loading