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

Adding small GPMSA verification example #614

Merged
merged 4 commits into from
Aug 2, 2017

Conversation

dmcdougall
Copy link
Member

Labelled as 'do not merge' until we iron out a potential issue with the normalisation of the scenario variables.

@dmcdougall dmcdougall added this to the v0.58.0 milestone Jul 27, 2017
@codecov
Copy link

codecov bot commented Jul 27, 2017

Codecov Report

Merging #614 into dev will decrease coverage by 0.02%.
The diff coverage is 55.41%.

Impacted file tree graph

@@            Coverage Diff            @@
##             dev     #614      +/-   ##
=========================================
- Coverage   75.1%   75.07%   -0.03%     
=========================================
  Files        310      311       +1     
  Lines      23288    23528     +240     
=========================================
+ Hits       17490    17664     +174     
- Misses      5798     5864      +66
Impacted Files Coverage Δ
test/test_gpmsa/scalar_pdf_small.C 55.41% <55.41%> (ø)
src/contrib/getpot/getpot.h 34.76% <0%> (+0.15%) ⬆️
src/gp/src/GPMSAOptions.C 77.59% <0%> (+10.92%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24867bf...3d5c1af. Read the comment docs.

This test checks that the pdf (both prior and likelihood) evaluations
between matlab and queso are consistent (to within 1e-5).

# -----
# GPMSA Options with data precision adjusted per Brian Williams
# -----
Copy link
Member

Choose a reason for hiding this comment

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

Don't a few of these just match the default options?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. I left them there just to be explicit.

//
// Regarding experimental data, the user should transformed it so that it has
// zero mean and variance one.

Copy link
Member

Choose a reason for hiding this comment

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

These comments are now outdated, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed them.

for (unsigned int j = 0; j < point.sizeLocal(); ++j)
solution_data >> point[j];

double log_likelihood;
Copy link
Member

Choose a reason for hiding this comment

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

Why all the intermediate variables?

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed.

}
solution_data.close();

double initial_diff_prior = expected_log_priors[0] - computed_log_priors[0];
Copy link
Member

Choose a reason for hiding this comment

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

This is because our log priors are correct only up to a constant, but we expect it to be the same constant at every evaluations? We should have a comment about that.

Copy link
Member Author

Choose a reason for hiding this comment

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

I forgot to address this comment in this PR. I did it in #615, though.

@dmcdougall
Copy link
Member Author

Should be good to do.

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