User model #1525

ferrine · 2016-11-12T22:56:02Z

I started preparation for implementing GLM model and I think good base class is necessary for that purpose. Also refer to #1524 and #1517.

ferrine · 2016-11-12T23:04:56Z

Do we really need compatible sklearn api? I think if well be a bit hard to make them friends
Are there any ideas of basic functionality of UserModel?

twiecki · 2016-11-14T09:27:12Z

@ferrine I don't think it's a priority to keep sklearn API. Can just be a starting point but if it's not helpful there's no reason to be beholden to it.

ferrine · 2016-11-15T00:02:09Z

Please review LinearComponent API and base class usage example

ferrine · 2016-11-15T04:24:26Z

Hmm, what's wrong with travis?

twiecki · 2016-11-15T09:24:11Z

pymc3/models/linear.py

+    init : dict - test_vals for coefficients
+    rvars : dict - random variables instead of creating new ones
+    """
+    def __init__(self, name, x, y, intercept=True, labels=None,


name is only required if you have multiple LinearComponents, right? Not sure it should be a required arg then.

twiecki · 2016-11-15T09:25:39Z

pymc3/models/base.py

+        """Shortcut to model"""
+        return modelcontext(None)
+
+    def new_var(self, name, dist, data=None, test_val=None):


I can see how this is a useful abstraction but it makes transcribing a PyMC3 model from the usual syntax to be a class a bit harder as it gives cognitive overhead. After all, the code below isn't doing so much that it warrants the indirection. Is there another way we can achieve that?

Unfortunately, there is no easier way to rename lots of variables in the model.

I also thought about a method like this for base class, but i'm not sure if it will be good for readability. Here it exists to avoid code duplication

ferrine · 2016-11-15T22:37:40Z

travis was failing because of that, waiting for merge this PR

springcoil · 2016-11-16T23:28:27Z

Are you ready to accept this @twiecki. I like the general idea.

ferrine · 2016-11-26T11:13:19Z

Though I don't think model specific plots are a part of this PR.

ferrine · 2016-11-26T21:53:31Z

Not in WIP. It's done.

Unfortunately functools wrapper fails when decorating built-in methods so I need to fix that improper behaviour. Some bad but needed tricks were implemented

ferrine · 2016-11-26T23:24:07Z

Travis plays bad tricks, definitely. I relied on his tests but in my new projects environment (python 2.7.12) model crushed.

ColCarroll

Looks really good! I'll have to look at the examples more, but seems fine to merge to me. I left some nitpicks that are probably better off as followup PRs.

ColCarroll · 2016-11-27T15:54:59Z

pymc3/model.py

+            return list.__contains__(self, item)
+
+    def __setitem__(self, key, value):
+        raise NotImplementedError('Not able to determine '


Can exception message be more specific? Something like "cannot set item value for treelist (tried to set {key} = {value})"?

I cannot set value at all. It inserts a value by some key, but in parent list there can be something else. I'm not sure if we need that, but this will probably cause unexpected behaviour.

Ah, I just meant the exception message could be more clear!

ColCarroll · 2016-11-27T16:00:21Z

pymc3/models/utils.py

+import theano.tensor as tt
+
+
+def any_to_tensor_and_labels(x, labels=None):


any chance for breaking up this logic a bit?

(just saw awesome test suite, but still comment still holds :) )

It looks like splitting out _input_to_array, _verify_labels, then you can rewrite this as

x = _input_to_array(x) labels = _verify_labels(labels) assert_shape_labels(x.shape, labels) return x, labels

labels can come from x, so they go together

ColCarroll · 2016-11-27T16:02:28Z

pymc3/models/utils.py

+    -------
+    (x, labels) - tensor and labels for it's columns
+    """
+    def assert_shape_labels(s, l):


looks like this is only used once

ColCarroll · 2016-11-27T16:04:46Z

pymc3/models/utils.py

+            raise ValueError('Cannot use scalars')
+        elif x.ndim == 1:
+            x = x[:, None]
+    # else: trust input


would love an explicit

else: pass

here

(and below)

ColCarroll · 2016-11-27T16:16:19Z

Also, there are a few uses of "it's" (="it is") in the docs that should change to "its" (possessive).

ferrine · 2016-11-28T00:11:00Z

BTW, gelato is comming. PyMC3 becomes really powerful

twiecki · 2016-11-28T04:38:05Z

Alright, I'll go ahead and merge this into the 3.1 branch. We'll merge it into master once we shipped 3.0. Thanks for the great contribution @ferrine.

twiecki · 2016-11-28T04:39:36Z

Just checking out Gelato, looks really interesting!

* Started to write Base class for pymc3.models * mode `add_var` to public api * Added some docstrings * Added some docstrings * added getitem and fixed a typo * added assertion check * added resolve var method * decided not to add resolve method * Added linear component * Docs fix * patsy's intercept is inited properly now * refactored code * updated docs * added possibility to init coefficients with random variables * added glm * refactored api, fixed formula init * refactored linear model, extended acceptable types * moved useful matrix and labels creation to utils file * code style * removed redundant evaluation of shape * refactored resolver for constructing matrix and labels * changed error message * changed signature of init * simplified utils any_to_tensor_and_labels code * tests for `any_to_tensor_and_labels` * added docstring for `any_to_tensor_and_labels` util * forgot to document return type in `any_to_tensor_and_labels` * refactored code for dict * dict tests fix(do not check labels there) * added access to random vars of model * added a shortcut for all variables so there is a unified way to get them * added default priors for linear model * update docs for linear * refactored UserModel api, made it more similar to pm.Model class * Lots of refactoring, tests for base class, more plain api design * deleted unused module variable * fixed some typos in docstring * Refactored pm.Model class, now it is ready for inheritance * Added documentation for Model class * Small typo in docstring * nested contains for treedict (needed for add_random_variable) * More accurate duplicate implementation of treedict/treelist * refactored treedict/treelist * changed `__imul__` of treelist * added `root` property and `isroot` indicator for base model * protect `parent` and `model` attributes from violation * travis' python2 did not fail on bad syntax(maybe it's too new), fixed * decided not to use functools wrapper Unfortunately functools wrapper fails when decorating built-in methods so I need to fix that improper behaviour. Some bad but needed tricks were implemented * Added models package to setup script * Refactor utils * Fix some typos in pm.model

ferrine added 4 commits November 13, 2016 01:28

Started to write Base class for pymc3.models

3c1d871

mode add_var to public api

0969955

Added some docstrings

d96d4ff

Added some docstrings

3d33fbf

added getitem and fixed a typo

fb4f6f5

ferrine added 4 commits November 15, 2016 00:09

added assertion check

e46c831

added resolve var method

1fbebfb

decided not to add resolve method

691e429

Added linear component

0f21836

ferrine added 2 commits November 15, 2016 03:05

Docs fix

4de2006

patsy's intercept is inited properly now

6092934

ferrine added 3 commits November 15, 2016 09:47

refactored code

40f7eb7

updated docs

d9d4e60

added possibility to init coefficients with random variables

39884c9

twiecki reviewed Nov 15, 2016

View reviewed changes

ferrine added 2 commits November 15, 2016 13:16

added glm

ab45f39

refactored api, fixed formula init

ed81564

ferrine added 6 commits November 16, 2016 03:14

refactored linear model, extended acceptable types

13885b1

moved useful matrix and labels creation to utils file

7afe41d

code style

625406e

removed redundant evaluation of shape

d09c6cb

refactored resolver for constructing matrix and labels

4ddce18

changed error message

33f1bfd

ferrine added 2 commits November 25, 2016 22:24

added root property and isroot indicator for base model

432a3b6

protect parent and model attributes from violation

129d05c

ferrine added 2 commits November 27, 2016 01:23

travis' python2 did not fail on bad syntax(maybe it's too new), fixed

2b04654

decided not to use functools wrapper

b27ec20

Unfortunately functools wrapper fails when decorating built-in methods so I need to fix that improper behaviour. Some bad but needed tricks were implemented

Added models package to setup script

29bc259

ColCarroll approved these changes Nov 27, 2016

View reviewed changes

ferrine added 2 commits November 27, 2016 19:42

Refactor utils

b403dd2

Fix some typos in pm.model

144f021

ColCarroll removed the WIP label Nov 27, 2016

twiecki changed the base branch from master to 3.1 November 28, 2016 04:37

twiecki merged commit 55c8ce6 into pymc-devs:3.1 Nov 28, 2016

This was referenced Nov 28, 2016

Submit BEST #1517

Closed

Base Model class for pymc3.models #1524

Closed

Relevant new PyMC3 features bambinos/bambi#55

Closed

ferrine deleted the user_model branch December 9, 2016 20:32

ferrine mentioned this pull request Mar 17, 2022

WIP Improve scoped models #5607

Merged

ricardoV94 mentioned this pull request Nov 24, 2023

Remove several deprecated model properties and deprecate new ones #7033

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

User model #1525

User model #1525

ferrine commented Nov 12, 2016

ferrine commented Nov 12, 2016 •

edited

Loading

twiecki commented Nov 14, 2016

ferrine commented Nov 15, 2016

ferrine commented Nov 15, 2016

twiecki Nov 15, 2016

ferrine Nov 15, 2016

twiecki Nov 15, 2016

ferrine Nov 15, 2016

ferrine Nov 15, 2016

ferrine commented Nov 15, 2016

springcoil commented Nov 16, 2016

ferrine commented Nov 26, 2016

ferrine commented Nov 26, 2016

ferrine commented Nov 26, 2016

ColCarroll left a comment

ColCarroll Nov 27, 2016

ferrine Nov 27, 2016

ColCarroll Nov 27, 2016

ColCarroll Nov 27, 2016

ColCarroll Nov 27, 2016

ColCarroll Nov 27, 2016

ferrine Nov 27, 2016

ColCarroll Nov 27, 2016

ColCarroll Nov 27, 2016

ColCarroll Nov 27, 2016

ColCarroll commented Nov 27, 2016

ferrine commented Nov 28, 2016 •

edited

Loading

twiecki commented Nov 28, 2016

twiecki commented Nov 28, 2016

		import theano.tensor as tt


		def any_to_tensor_and_labels(x, labels=None):

User model #1525

User model #1525

Conversation

ferrine commented Nov 12, 2016

ferrine commented Nov 12, 2016 • edited Loading

twiecki commented Nov 14, 2016

ferrine commented Nov 15, 2016

ferrine commented Nov 15, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ferrine commented Nov 15, 2016

springcoil commented Nov 16, 2016

ferrine commented Nov 26, 2016

ferrine commented Nov 26, 2016

ferrine commented Nov 26, 2016

ColCarroll left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ColCarroll commented Nov 27, 2016

ferrine commented Nov 28, 2016 • edited Loading

twiecki commented Nov 28, 2016

twiecki commented Nov 28, 2016

ferrine commented Nov 12, 2016 •

edited

Loading

ferrine commented Nov 28, 2016 •

edited

Loading