Skip to content

Added code to fit_line_emcee.py - #1

Open
MaiAdityaHun wants to merge 1 commit into
PhysicistSouravDas:mainfrom
MaiAdityaHun:main
Open

Added code to fit_line_emcee.py#1
MaiAdityaHun wants to merge 1 commit into
PhysicistSouravDas:mainfrom
MaiAdityaHun:main

Conversation

@MaiAdityaHun

Copy link
Copy Markdown

No description provided.

Comment thread fit_line_emcee.py

lp=log_prior(theta)
if (lp==-np.inf):
return

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@MaiAdityaHun
Only writing return actually returns a None when this if condition satisfies, which will throw an error while running emcee. emcee always expects a float kind of data type to be returned by log_probability. Consider changing the if block with the following:

...
if not np.isfinite(lp):
    return -np.inf
...

Comment thread fit_line_emcee.py
if (lp==-np.inf):
return
else:
return log_prior(theta)+log_likelihood(theta, x, y, yerr)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The else here is redundant, since a function only returns once, and whatever is returned, the function doesn't see any lines within it below the return. For example:

def f(x):
    if x >=0:
        return x**3
    else:  # when x < 0
        return x**2

The above function can be effectively written as:

def f(x):
    if x >=0:
        return x**3
    # when x < 0
    return x**2

And, in your last return statement, you are using log_prior(theta) again, which will again call the function and compute the value, but few lines above, you have already defined lp=log_prior(theta), which already computes the prior for a given theta and stores in lp. So, you can just write return lp + log_likelihood(theta, x, y, yerr). The way you have done is absolutely correct, but will waste computation if computing it is computationally costly.

@PhysicistSouravDas PhysicistSouravDas left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@MaiAdityaHun thanks for looking into the problem and submitting the PR. I have highlighted a few very minor changes which will make your PR better. Can you resolve those, commit a change and push it again? You can take your time.

Regards

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