-
-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow interdependent initial points from same OpFromGraph node #7569
base: main
Are you sure you want to change the base?
Conversation
It is not part of the API that variables must be registered in topological order
95d3dde
to
285263d
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7569 +/- ##
=======================================
Coverage 92.83% 92.83%
=======================================
Files 106 106
Lines 17669 17681 +12
=======================================
+ Hits 16403 16415 +12
Misses 1266 1266
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@@ -187,6 +193,40 @@ def test_string_overrides_work(self): | |||
assert np.isclose(iv["B_log__"], 0) | |||
assert iv["C_log__"] == 0 | |||
|
|||
@pytest.mark.parametrize("reverse_rvs", [False, True]) | |||
def test_dependent_initval_from_OFG(self, reverse_rvs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this test indicative of why this features is need for the other PR?
This is needed to allow proper initival support of Marginalized variables in pymc-devs/pymc-experimental#388, where multiple interdependent variables may become collapsed in a single node. We want initial values replacements to still propagate across these dependencies.
To achieve that the initial point machinery now makes use of
toposort_replace
(instead of assuming model variables are provided in a topological order) and the utility itself is expanded to untie variables coming from the same OpFromGraph node📚 Documentation preview 📚: https://pymc--7569.org.readthedocs.build/en/7569/