fix autoreparam because dims are no longer static #363

ferrine · 2024-07-19T10:02:09Z

No description provided.

ricardoV94 · 2024-07-19T10:52:25Z

pymc_experimental/model/transforms/autoreparam.py

-    except NotConstantValueError:
-        raise ValueError("Size should be static for autoreparametrization.")
+    _, size, *_ = node.inputs
+    eval_size = size.eval()


How is this better?

Can you do constant_fold([rv_shape], raise_if_not_constant=False)?

I remember hitting missing input errors, but I was doing plain eval

Hmm the problem is you do need constant values with the shared variable

ok, I reverted this due to this uncertainty. I only need this shape once, the computation of it can be addressed better. I saw in one place you did manual function compute with linker="py" and no optimizations, this would be suitable here as well but I do not want to duplicate code

You can do eval(mode="FAST_COMPILE") if you want it to be fast. Still does some rewrites but not much and avoids C compilation

pymc_experimental/model/transforms/autoreparam.py

ricardoV94 · 2024-07-19T11:06:00Z

pymc_experimental/model/transforms/autoreparam.py

-        [rv_shape] = constant_fold([rv.shape])
-    except NotConstantValueError:
-        raise ValueError("Size should be static for autoreparametrization.")
+    [rv_shape] = constant_fold([rv.shape], raise_if_not_constant=False)


Ignore my idea, this won't work for you. When it can't constant_fold, it returns the best graph it can but you still need constants for np.zeros

ricardoV94 · 2024-07-23T10:50:38Z

pymc_experimental/model/transforms/autoreparam.py

-        raise ValueError("Size should be static for autoreparametrization.")
+    _, size, *_ = node.inputs
+    eval_size = size.eval(mode="FAST_COMPILE")
+    if eval_size is not None:


eval_size will never be None

Wait why are you evaling size instead of rv.shape?

rv.shape.eval()

That's the one you should use

I was afraid I get the dependence on the original RV, ah, I do not seem to do

If you use eval you can only get a numerical output

Do you need shape of transformed or original? If it is the transformed, you could request model.initial_point before starting the dispatch process?

Alternatively we should be able to use shape inference but may need to implement the infer_shape of the ModelVariable. Then the helper infer_static_shape, possibly followed by an eval, should do the job and not complain about value variables because those shouldn't be in the graph after infer_shape

Can you point me to which test failed when you tried rv.shape.eval()?

this one: https://github.com/pymc-devs/pymc-experimental/actions/runs/10058239228/job/27800877978#step:5:1461

Do you need shape of transformed or original? If it is the transformed, you could request model.initial_point before starting the dispatch process?

This looks like an interesting solution

This looks like an interesting solution

model.initial_point()

Seems to give a point in the sampling space, I need the untransformed one

ricardoV94 · 2024-07-23T10:51:21Z

Looks good to me, just a useless eval_size check perhaps from when I told you to try the constant_fold function? Shouldn't be needed

ferrine · 2024-07-23T11:36:26Z

Looks good to me, just a useless eval_size check perhaps from when I told you to try the constant_fold function? Shouldn't be needed

I'll have a branching condition to support both then

ricardoV94 · 2024-07-23T11:45:50Z

Looks good to me, just a useless eval_size check perhaps from when I told you to try the constant_fold function? Shouldn't be needed

I'll have a branching condition to support both then

I think you cannot use the symbolic shape because you need to define the shared variable with a concrete size? Or is it not required?

This reverts commit 557caec.

This reverts commit 0ca9f68.

ricardoV94

This was a tricky one!

ricardoV94 · 2024-07-24T08:42:54Z

Oh still failing :( ?

ferrine · 2024-07-24T14:28:49Z

Yeah, I still get this input error

FAILED tests/model/transforms/test_autoreparam.py::test_set_truncate - pytensor.graph.utils.MissingInputError: Input 1 (s_log__) of the graph (indices start from 0), used to compute ModelFreeRV{transform=<pymc.logprob.transforms.LogTransform object at 0x7f4afe540550>}(s, s_log__), was not provided and not given a value. Use the PyTensor flag exception_verbosity='high', for more information on this error.

ferrine · 2024-07-24T14:29:48Z

@ricardoV94 should I revert back to an initial solution? Without the fix, autoreparam can't be used on any model with dims(

ricardoV94 · 2024-07-24T15:14:40Z

Can you dprint the graph to see why the value variables are there?

ferrine · 2024-07-25T08:59:18Z

This is how I do the replacement:

pymc-experimental/pymc_experimental/model/transforms/autoreparam.py

Line 411 in a9f582c

old = memo[model.named_vars[name]]

This reverts commit a9f582c.

ferrine · 2024-07-26T07:53:25Z

I reverted to the solution which is working but not ideal. I'll open an issue to address the future-proof refactoring after we resolve this bug

ricardoV94 · 2024-07-26T10:36:31Z

Cab you add an xfail test with the case we know to be broken now?

ricardoV94 · 2024-07-26T10:37:24Z

pymc_experimental/model/transforms/autoreparam.py

@@ -174,15 +176,18 @@ def vip_reparam_node(
 ) -> Tuple[ModelDeterministic, ModelNamed]:
    if not isinstance(node.op, RandomVariable | SymbolicRandomVariable):
        raise TypeError("Op should be RandomVariable type")


Do you want to raise NotImplementedError for op.ndim_supp>0?

Not sure if for those you would want one lambda per shape or size, mentioning because those will always be different for multivariate RVs

it will not pass the dispatch anyway, it is not needed

ndim_supp>0, will never be supported I think, but I'm not sure, maybe it will, e.g. Dirichlet, there are some reparameterizations with Gamma https://stats.stackexchange.com/questions/548620/reparameterization-trick-for-the-dirichlet-distribution

one lambda per shape or size

Per size for sure 100%

I believe that if I work with multivariate rv, it will be the only way to have one lambda per size (one per independent draw)

pymc_experimental/model/transforms/autoreparam.py

Co-authored-by: Ricardo Vieira <[email protected]>

ferrine · 2024-07-26T17:20:54Z

Can you add an xfail test with the case we know to be broken now?

Can you please elaborate on this?

If size is None, lambda should be scalar, no?

PS I wish github has threads

ferrine force-pushed the fix/autoreparam branch 2 times, most recently from 7bbb7d8 to 28a670b Compare July 19, 2024 10:44

ricardoV94 reviewed Jul 19, 2024

View reviewed changes

pymc_experimental/model/transforms/autoreparam.py Show resolved Hide resolved

ricardoV94 reviewed Jul 19, 2024

View reviewed changes

pymc_experimental/model/transforms/autoreparam.py Show resolved Hide resolved

ricardoV94 reviewed Jul 19, 2024

View reviewed changes

ferrine requested a review from ricardoV94 July 19, 2024 13:14

ferrine force-pushed the fix/autoreparam branch from 9ae000f to 60a04ff Compare July 19, 2024 14:28

ricardoV94 reviewed Jul 23, 2024

View reviewed changes

ferrine added 7 commits July 24, 2024 07:33

fix autoreparam because dims are no longer static

c87db47

fix: Handle case with size passed as None

2d80fdf

fix: use constant_fold instead of manual eval

9ec41cd

Revert "fix: use constant_fold instead of manual eval"

949d2e9

This reverts commit 557caec.

fix: do not trigger C compile when evaluating shape

a3907f0

Evaluate the rv.shape directly

d2b3584

Revert "Evaluate the rv.shape directly"

c2cdbdc

This reverts commit 0ca9f68.

ferrine force-pushed the fix/autoreparam branch from 5b0a796 to c2cdbdc Compare July 24, 2024 07:33

ferrine added 2 commits July 24, 2024 07:38

test: Check both variants of static and not static shape

75c98ec

refactor: Use infer_static_shape from pytensor

a9f582c

ferrine requested a review from ricardoV94 July 24, 2024 07:48

ricardoV94 approved these changes Jul 24, 2024

View reviewed changes

Revert "refactor: Use infer_static_shape from pytensor"

82a1a24

This reverts commit a9f582c.

ricardoV94 reviewed Jul 26, 2024

View reviewed changes

pymc_experimental/model/transforms/autoreparam.py Show resolved Hide resolved

Update pymc_experimental/model/transforms/autoreparam.py

d5355a5

Co-authored-by: Ricardo Vieira <[email protected]>

add failing test for all cases of how shape can go

bcdea46

ricardoV94 approved these changes Jul 26, 2024

View reviewed changes

ferrine merged commit 7ce4ac8 into main Jul 27, 2024
8 checks passed

ricardoV94 deleted the fix/autoreparam branch October 4, 2024 07:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix autoreparam because dims are no longer static #363

fix autoreparam because dims are no longer static #363

ferrine commented Jul 19, 2024

ricardoV94 Jul 19, 2024 •

edited

Loading

ferrine Jul 19, 2024 •

edited

Loading

ricardoV94 Jul 19, 2024 •

edited

Loading

ferrine Jul 19, 2024

ricardoV94 Jul 19, 2024

ricardoV94 Jul 19, 2024

ricardoV94 Jul 23, 2024

ferrine Jul 23, 2024

ricardoV94 Jul 23, 2024 •

edited

Loading

ferrine Jul 23, 2024 •

edited

Loading

ricardoV94 Jul 23, 2024

ricardoV94 Jul 23, 2024 •

edited

Loading

ricardoV94 Jul 23, 2024

ferrine Jul 23, 2024

ferrine Jul 23, 2024

ferrine Jul 23, 2024

ricardoV94 commented Jul 23, 2024

ferrine commented Jul 23, 2024

ricardoV94 commented Jul 23, 2024 •

edited

Loading

ricardoV94 left a comment

ricardoV94 commented Jul 24, 2024

ferrine commented Jul 24, 2024

ferrine commented Jul 24, 2024

ricardoV94 commented Jul 24, 2024

ferrine commented Jul 25, 2024

ferrine commented Jul 26, 2024

ricardoV94 commented Jul 26, 2024 •

edited

Loading

ricardoV94 Jul 26, 2024 •

edited

Loading

ferrine Jul 26, 2024

ferrine Jul 26, 2024 •

edited

Loading

ferrine Jul 26, 2024 •

edited

Loading

ferrine commented Jul 26, 2024 •

edited

Loading

fix autoreparam because dims are no longer static #363

fix autoreparam because dims are no longer static #363

Conversation

ferrine commented Jul 19, 2024

ricardoV94 Jul 19, 2024 • edited Loading

Choose a reason for hiding this comment

ferrine Jul 19, 2024 • edited Loading

Choose a reason for hiding this comment

ricardoV94 Jul 19, 2024 • edited Loading

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

ricardoV94 Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

ferrine Jul 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ricardoV94 Jul 23, 2024 • edited Loading

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

ricardoV94 commented Jul 23, 2024

ferrine commented Jul 23, 2024

ricardoV94 commented Jul 23, 2024 • edited Loading

ricardoV94 left a comment

Choose a reason for hiding this comment

ricardoV94 commented Jul 24, 2024

ferrine commented Jul 24, 2024

ferrine commented Jul 24, 2024

ricardoV94 commented Jul 24, 2024

ferrine commented Jul 25, 2024

ferrine commented Jul 26, 2024

ricardoV94 commented Jul 26, 2024 • edited Loading

ricardoV94 Jul 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ferrine Jul 26, 2024 • edited Loading

Choose a reason for hiding this comment

ferrine Jul 26, 2024 • edited Loading

Choose a reason for hiding this comment

ferrine commented Jul 26, 2024 • edited Loading

ricardoV94 Jul 19, 2024 •

edited

Loading

ferrine Jul 19, 2024 •

edited

Loading

ricardoV94 Jul 19, 2024 •

edited

Loading

ricardoV94 Jul 23, 2024 •

edited

Loading

ferrine Jul 23, 2024 •

edited

Loading

ricardoV94 Jul 23, 2024 •

edited

Loading

ricardoV94 commented Jul 23, 2024 •

edited

Loading

ricardoV94 commented Jul 26, 2024 •

edited

Loading

ricardoV94 Jul 26, 2024 •

edited

Loading

ferrine Jul 26, 2024 •

edited

Loading

ferrine Jul 26, 2024 •

edited

Loading

ferrine commented Jul 26, 2024 •

edited

Loading