search

mikajoh / tidystm

Extract effects from estimateEffect in the stm package
47 stars 4 forks source link

Issue with topic selection #1

Closed joshgreen4564 closed 6 years ago

joshgreen4564 commented 6 years ago

Currently, the package throws an error when you specify the topics option with anything that is not a concurrent sequence starting at 1. For example, if there are 10 topics, you could use the topics option to select 1:3 or 1:7, but the function breaks when you use 4:8 or c(3,5,7) etc.

The issue has an easy fix. The sims object should be created this way:

sims <- cthis$cmatrix %*% t(simbetas[[which(x$topics == i)]])

not this way:

sims <- cthis$cmatrix %*% t(simbetas[[which(x$topics == topics[i])]])

You also need to change the labels selection piece to:

label = labels[which(x$topics == i)]

mikajoh commented 6 years ago

Thanks! I implemented you suggestion. Does it work now?

joshgreen4564 commented 6 years ago

Hey! What a quick reply!

So there’s still an issue with the labels.... though I’m not that sure how to fix it. My code fixes the actual effects extraction if you subset the topics in the estimateEffects() object or in your function, but it only fixes the if you have chosen the topic subset in the estimateEffect() object and not using your topics option. You get incorrect and NA labels if you try and subset topics with the topics option.

If you’d like, I can try and think through a solution. Else, I think you might be better off removing the topics option altogether as a user defined option and just making it clear in the readme or examples that the topic subsetting has to be done in the estimateEffect() object. Let me know what I can do to help!

Josh

On Mar 2, 2018, at 7:53 AM, Mikael Poul Johannesson notifications@github.com wrote:

Thanks! I implemented you suggestion. Does it work now?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

joshgreen4564 commented 6 years ago

I fixed it!

Keep both my fixes from before and also change topics = topics to topics =x$topics in the stm::createlabels() function.

That should do it :)

Josh

On Mar 2, 2018, at 7:53 AM, Mikael Poul Johannesson notifications@github.com wrote:

Thanks! I implemented you suggestion. Does it work now?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

mikajoh commented 6 years ago

Thanks, nice work. Try now!

mikajoh commented 6 years ago

@joshgreen4564 Could you confirm if is this works for you?

joshgreen4564 commented 6 years ago

I haven’t have the chance to reinstall the latest version (sorry I’m on vacation), but I can confirm that the change does fix the function independently!

Josh

On Mar 4, 2018, at 4:26 PM, Mikael Poul Johannesson notifications@github.com wrote:

@joshgreen4564 Could you confirm if is this works for you?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

mikajoh commented 6 years ago

No worries, enjoy your vacation! Again, thanks.

Best,

Mikael