Use Scalar op property for monotonic increasing / decreasing instead of redefining them.

[ricardoV94]

pymc/pymc/logprob/transforms.py

Lines 233 to 234 in 6182801

	MONOTONICALLY_INCREASING_OPS = (Exp, Log, Add, Sinh, Tanh, ArcSinh, ArcCosh, ArcTanh, Erf, Sigmoid)
	MONOTONICALLY_DECREASING_OPS = (Erfc, Erfcx)

We recently added them in pymc-devs/pytensor#2053

We can remove those manual definition, and instead check for the properties in the part of the code that was accessing that list. Requires a bit of refactoring

[ahmedbilal9]

Hi, I'd like to take this one. Looks like both tuples are used in measurable_transform_logcdf and measurable_transform_icdf, so the plan is to swap the isinstance checks for direct access to the new monotonic_increasing / monotonic_decreasing attributes from pytensor#2053, then drop the tuple definitions. Will put up a draft PR once I've got it running locally.

[ahmedbilal9]

Looked into this — the monotonic_increasing/monotonic_decreasing props from pytensor#2053 are only on the v3 branch and aren't in the released pytensor (2.38.2) that PyMC currently depends on. Is this issue blocked on a future pytensor v3 release, or is there a different approach I'm missing? Happy to come back to it once pytensor v3 is cut.

[ricardoV94]

pymc dev branch (v6) is built against pytensor v3 branch so you can already use it. Releases will happen soon, but you can get started if you want

[ahmedbilal9]

pymc dev branch (v6) is built against pytensor v3 branch so you can already use it. Releases will happen soon, but you can get started if you want

Great, thanks — I'll work against the v6 branch then.

[ahmedbilal9]

Got the v6 branch set up. Quick question on approach: Add, Mul, and Pow don't have the monotonic_increasing/monotonic_decreasing properties (they're on UnaryScalarOp only). Mul and Pow are already handled separately in the code with their own branches, but Add is currently in the MONOTONICALLY_INCREASING_OPS tuple. Two options:

Keep Add as a special case in pymc/logprob/transforms.py (one extra isinstance check), use the property for everything else.
Open a separate pytensor PR to add the property to Add first.

I was thinking option 1 since it keeps the change scoped to one repo, but happy to do 2 if you prefer a cleaner end state.

[ricardoV94]

Treat them separately, mul and pow are definitely not monotonic, as it depends on the value of other parameters. Add you could stretch to I guess, but doesn't buy us much

[ricardoV94]

@jessegrabowski should we add it to Add?

[jessegrabowski]

Yeah Add seems like a free win. Mul and Pow we'd have to add some logic, let's wait and see if these tags prove out their value before we jump through all those hoops.

[ricardoV94]

Mul and Pow are not statically monotonic

[jessegrabowski]

Yeah I agree.