Skip to content

Allow multivariate components to share latent states #558

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

Merged
merged 11 commits into from
Aug 5, 2025

Conversation

jessegrabowski
Copy link
Member

@jessegrabowski jessegrabowski commented Aug 3, 2025

This PR allows multivariate components to share latent states between observed states. For example, if we want two states to have a shared component and an idiosyncratic component, as in:

$$ \begin{align} a_t &= x_t + y_t \\ b_t &= x_t + z_t \\ \end{align} $$

This can be accomplished as follows:

    shared_level = st.LevelTrendComponent(
        name="shared_level",
        order=1,
        innovations_order=1,
        observed_state_names=["a", "b"],
        share_states=True,
    )
    individual_level = st.LevelTrendComponent(
        name="individual_level",
        order=1,
        innovations_order=1,
        observed_state_names=["a", "b"],
    )

    mod = (shared_level + individual_level).build(verbose=False)

This will require a refactor for each component to add the shared_states argument. Here's the TODO list:

  • LevelTrendComponent
  • AutoregressiveComponent
  • CycleComponent
  • RegressionComponent
  • TimeSeasonality
  • FrequencySeasonality
  • MeasurementError

@jessegrabowski jessegrabowski added enhancements New feature or request help wanted Extra attention is needed statespace labels Aug 3, 2025
@AlexAndorra
Copy link
Contributor

This is 🔥
We probably wanna do it for Noise too (added it as an item in the description)

@AlexAndorra
Copy link
Contributor

@jessegrabowski, while playing around with that, I realize it's breaking _hidden_states_from_data in a subtle way (the component extraction logic assumes it can always split observation indices into equal groups for each endogenous variable, but when share_states=True, there's only one set of shared states instead of separate states per endogenous variable).

I fixed it, but it'd be better if we stored the share_states boolean into _component_info (it makes the logic much simpler). This is a simple implementation, that I demoed here (ignore the changes for seasonality, which are just the ones you implemented here; focus on how share_states is passed between objects, and then to Component in core.py).

If we do that, we'd just need to make sure each component passes its share_states flag from its __init__ to its super call. LMK what you think!

@jessegrabowski
Copy link
Member Author

What is noise? MeasurementError?

@jessegrabowski
Copy link
Member Author

@jessegrabowski, while playing around with that, I realize it's breaking _hidden_states_from_data in a subtle way (the component extraction logic assumes it can always split observation indices into equal groups for each endogenous variable, but when share_states=True, there's only one set of shared states instead of separate states per endogenous variable).

I am getting sick of this function. I'm thinking about a big refactor that would take advantage of the computational graph

Copy link
Contributor

@AlexAndorra AlexAndorra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good guys! Almost ready to merge: we need to pass the share_states flag to the super call in each component. I'll add that, and fix the merge conflict

@jessegrabowski
Copy link
Member Author

Looks good guys! Almost ready to merge: we need to pass the share_states flag to the super call in each component. I'll add that, and fix the merge conflict

Why?

@jessegrabowski
Copy link
Member Author

Also please rebase instead of merging

@AlexAndorra
Copy link
Contributor

AlexAndorra commented Aug 5, 2025

Looks good guys! Almost ready to merge: we need to pass the share_states flag to the super call in each component. I'll add that, and fix the merge conflict

Why?

Because I need it here, in core.py @jessegrabowski -- well, at least I think I do, but LMK if I'm over-complicating this

Copy link
Contributor

@AlexAndorra AlexAndorra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good now!

@AlexAndorra AlexAndorra merged commit b5aa9bf into pymc-devs:main Aug 5, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancements New feature or request help wanted Extra attention is needed statespace
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants