Skip to content

Conversation

@divyanshu132
Copy link

@divyanshu132 divyanshu132 commented Mar 9, 2019

Fixes #3372

Previously:

logpt:
    Theano scalar of log-probability of the model

logp_nojact:
    Theano scalar of log-probability of the model

Now:

logpt: 
    Theano scalar of log-probability of the model
logp_nojact: 
    Theano scalar of log-probability of the model but without the jacobian 
    if transformed Random Variable is presented.

@divyanshu132
Copy link
Author

@junpenglao please review.

def logp_nojact(self):
"""Theano scalar of log-probability of the model"""
"""Theano scalar of log-probability of the model but without the jacobian
if transformed Random Variable is presented.
Copy link
Member

Choose a reason for hiding this comment

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

Does this mean that the Jacobian will be present if there are no transformed random variables? That is how I understand the sentence.

Copy link
Member

Choose a reason for hiding this comment

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

If there are no transformed variables there is no need for Jacobian correction, hence logp_nojact will be the same as logpt
This should be rephrased to make it clearer indeed.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if transformed Random Variable is presented.
if transformed Random Variable is presented.
Note that If there is no transformed variable in the model, logp_nojact
will be the same as logpt as there is no need for Jacobian correction.

def logp_nojact(self):
"""Theano scalar of log-probability, excluding jacobian terms."""
"""Theano scalar of log-probability of the model but without the jacobian
if transformed Random Variable is presented.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if transformed Random Variable is presented.
if transformed Random Variable is presented.
Note that If there is no transformed variable in the model, logp_nojact
will be the same as logpt as there is no need for Jacobian correction.

def logp_nojact(self):
"""Theano scalar of log-probability of the model"""
"""Theano scalar of log-probability of the model but without the jacobian
if transformed Random Variable is presented.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if transformed Random Variable is presented.
if transformed Random Variable is presented.
Note that If there is no transformed variable in the model, logp_nojact
will be the same as logpt as there is no need for Jacobian correction.

@chang111
Copy link
Contributor

@divyanshu132 Do you still follow up on this question? If you don't have time, I can submit it again on your pr. I think this problem is very small, but changing it as soon as possible is very friendly to the user.

@divyanshu132
Copy link
Author

@chang111 Actually I'm busy with some other organization, feel free to take it!

@chang111
Copy link
Contributor

@divyanshu132 Thanks You very much. I will push pr through your branch for you have made so much attempt for it.

@chang111
Copy link
Contributor

@junpenglao Can you tell me how to pull request on other's branch. I am sorry for that I do not know how to do it.

@junpenglao
Copy link
Member

fork the branch divyanshu132:fix-nojac in this case, make changes, and commit+push your changes directly to this branch

@chang111
Copy link
Contributor

Thank you, I will try it today.

@junpenglao junpenglao closed this Mar 26, 2019
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.

4 participants