Skip to content
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

Enforce positivity on sigmas in SEM model #783

Merged
merged 3 commits into from
Apr 6, 2025

Conversation

NathanielF
Copy link
Contributor

@NathanielF NathanielF commented Apr 3, 2025

Model Specification Improvement on SEM mediation model

Related to this issue: #782

Helpful links


📚 Documentation preview 📚: https://pymc-examples--783.org.readthedocs.build/en/783/

Signed-off-by: Nathaniel <NathanielF@users.noreply.github.com>
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Signed-off-by: Nathaniel <NathanielF@users.noreply.github.com>
@NathanielF NathanielF requested a review from fonnesbeck April 4, 2025 00:12
@NathanielF
Copy link
Contributor Author

Tagging you @fonnesbeck as i think you reviewed the original. This is a small improvement to the final SEM mediation model in light of the issue raised on PyMC discourse.

I've also changed the prior on the variances from Inverse Gamma to Gamma to match the default in the Blavaan package.

Copy link

review-notebook-app bot commented Apr 4, 2025

View / edit / reply to this conversation on ReviewNB

fonnesbeck commented on 2025-04-04T14:58:54Z
----------------------------------------------------------------

typo: "...data IN psychometrics ..."


NathanielF commented on 2025-04-06T21:01:52Z
----------------------------------------------------------------

Thanks, fixed.

Copy link

review-notebook-app bot commented Apr 4, 2025

View / edit / reply to this conversation on ReviewNB

fonnesbeck commented on 2025-04-04T14:58:55Z
----------------------------------------------------------------

Perhaps briefly say where the notation comes from -- what is Ksi? An acronym?


NathanielF commented on 2025-04-06T21:03:12Z
----------------------------------------------------------------

I've added a note to clarify that the notation reflects the Levy and Mislevy book and additionally flagged that the greek notation versus Ksi distinguishes the latent factor draws from the other parameters.

Copy link

review-notebook-app bot commented Apr 4, 2025

View / edit / reply to this conversation on ReviewNB

fonnesbeck commented on 2025-04-04T14:58:56Z
----------------------------------------------------------------

If you are interested in making mu more compact:

lambdas = pm.math.concatenate([lambdas_1, lambdas_2])
ksi_cols = pm.math.stack([ksi[obs_idx, 0], ksi[obs_idx, 0], ksi[obs_idx, 0], 
                            ksi[obs_idx, 1], ksi[obs_idx, 1], ksi[obs_idx, 1]]).T
mu = pm.Deterministic("mu", tau + ksi_cols * lambdas) 

NathanielF commented on 2025-04-06T21:04:12Z
----------------------------------------------------------------

Decided to stick with the less compact notation.

Copy link

review-notebook-app bot commented Apr 4, 2025

View / edit / reply to this conversation on ReviewNB

fonnesbeck commented on 2025-04-04T14:58:57Z
----------------------------------------------------------------

"Same scale" meaning within an order of magnitude?


Copy link

review-notebook-app bot commented Apr 4, 2025

View / edit / reply to this conversation on ReviewNB

fonnesbeck commented on 2025-04-04T14:58:57Z
----------------------------------------------------------------

Add an energy plot as part of the convergence-checking workflow?


Copy link

review-notebook-app bot commented Apr 4, 2025

View / edit / reply to this conversation on ReviewNB

fonnesbeck commented on 2025-04-04T14:58:58Z
----------------------------------------------------------------

These are really nice. I wish this was the default behavior for plot_ppc.


NathanielF commented on 2025-04-06T21:04:43Z
----------------------------------------------------------------

I could look into adding this to arviz.

Copy link

review-notebook-app bot commented Apr 4, 2025

View / edit / reply to this conversation on ReviewNB

fonnesbeck commented on 2025-04-04T14:58:59Z
----------------------------------------------------------------

"in-apt"? Is this short for inappropriate?


NathanielF commented on 2025-04-06T21:05:02Z
----------------------------------------------------------------

Yes, just changed for inappropriate

Copy link
Member

@fonnesbeck fonnesbeck left a comment

Choose a reason for hiding this comment

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

LGTM. A couple of typos and comments that you are free to ignore.

@NathanielF
Copy link
Contributor Author

Thanks @fonnesbeck will amend next week! Enjoy your weekend!

Signed-off-by: Nathaniel <NathanielF@users.noreply.github.com>
Copy link
Contributor Author

Thanks, fixed.


View entire conversation on ReviewNB

Copy link
Contributor Author

I've added a note to clarify that the notation reflects the Levy and Mislevy book and additionally flagged that the greek notation versus Ksi distinguishes the latent factor draws from the other parameters.


View entire conversation on ReviewNB

Copy link
Contributor Author

Decided to stick with the less compact notation.


View entire conversation on ReviewNB

Copy link
Contributor Author

I could look into adding this to arviz.


View entire conversation on ReviewNB

Copy link
Contributor Author

Yes, just changed for inappropriate


View entire conversation on ReviewNB

@NathanielF
Copy link
Contributor Author

NathanielF commented Apr 6, 2025

Updated with those changes there @fonnesbeck .

Would you be able to merge whenever is convenient, i don't have those permissions on this repo.

Thanks again!

@fonnesbeck fonnesbeck merged commit 8db7cc4 into pymc-devs:main Apr 6, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants