search

libqueso / queso

QUESO is a C++ library for doing uncertainty quantification. QUESO stands for Quantification of Uncertainty for Estimation, Simulation and Optimization.
Other
57 stars 40 forks source link

GPMSA functional branch #626

Closed roystgnr closed 7 years ago

roystgnr commented 7 years ago

This isn't ready to merge yet, because it's still missing big chunks of implementation, but it should be ready to look at, so I don't hit @dmcdougall with one giant pile of commits out of the blue later. I'll keep pushing further commits here.

In particular, the APIs are there now for specifying a mesh for functional data, for supplying point locations for functional data in experimental outputs, and for mixing multiple functional data fields with multivariate data.

roystgnr commented 7 years ago

Oh, and if anyone actually pulls this rather than just examining it via the web interface, be ready for lots of rebase+force-push to deal with. So far I've had "rebase to sync with Damon's fixes", "rebase to fixup a dbg-mode bug since I'd forgotten to test in dbg mode", and I may shortly have a "rebase to fix a bug that seems to have cropped up when I didn't correctly fix conflicts with Damon's fixes".

codecov[bot] commented 7 years ago

Codecov Report

Merging #626 into dev will increase coverage by 0.54%. The diff coverage is 82.12%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #626      +/-   ##
==========================================
+ Coverage   74.84%   75.38%   +0.54%     
==========================================
  Files         312      318       +6     
  Lines       23788    24270     +482     
==========================================
+ Hits        17804    18297     +493     
+ Misses       5984     5973      -11
Impacted Files Coverage Δ
test/test_gpmsa/test_gpmsa_vector.C 97.48% <ø> (ø) :arrow_up:
test/test_gpmsa/scalar_pdf_large.C 55.88% <ø> (+0.56%) :arrow_up:
test/test_gpmsa/scalar_pdf_small.C 54.85% <ø> (+0.57%) :arrow_up:
test/test_Regression/test_gpmsa_cobra.C 97.74% <ø> (ø) :arrow_up:
src/gp/inc/SimulationOutputMesh.h 0% <0%> (ø)
src/gp/inc/SimulationOutputPoint.h 100% <100%> (ø)
test/unit/tensor_product_mesh.C 100% <100%> (ø)
src/gp/src/SimulationOutputMesh.C 50% <50%> (ø)
src/gp/src/GPMSAOptions.C 71.76% <60.51%> (-6.02%) :arrow_down:
src/gp/src/TensorProductMesh.C 85.8% <85.8%> (ø)
... and 17 more

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 c02840f...29f16d5. Read the comment docs.

roystgnr commented 7 years ago

We have got to turn those codecov red X's into something else, in the icons next to commit hashes. I know admitting I have a problem is the first step, and I admit I have a problem with test coverage, but surely there's a huge difference between "PR is bad because it breaks tests" (which should get a red X) and "PR is bad because it adds features before adding test coverage" (which, I dunno, frowny face?)

roystgnr commented 7 years ago

Passed "make check" on both my systems and Travis' after the first rebase; let's see if Travis likes the second rebase too.

roystgnr commented 7 years ago

This is now at the point where it's not impossible that a functional case might work.

(In practice, I'm sure the first functional case I try will segfault if optimized or throw an exception if GLIBCXX_DEBUG; we'll find out for sure and hopefully fix what ought to be that easiest-to-hunt-down class of errors soon)

roystgnr commented 7 years ago

Ah, there's still a clear bug here - I'm calling output normalization functions with vector indices rather than variable numbers. I'll fix that.

roystgnr commented 7 years ago

Ah, hell, that was another merge conflict with Damon's fixes. Rebasing and manually fixing now.

roystgnr commented 7 years ago

And there's another merge conflict. We definitely ought to just get this into dev ASAP.

roystgnr commented 7 years ago

Okay, two fixups to previous commits, and it's compiling and passing regression tests for me again.

I'm still hoping to merge this as soon as we get the Wy fix in, but it can wait if @dmcdougall and others want more time to review. At the rate I'm rebasing I'm soon to dethrone Harry Turtledove as the master of alternate history.

roystgnr commented 7 years ago

This isn't actually quite ready yet - it doesn't even compile (well, link) with --enable-boost-program-options, apparently because I added a couple new use cases without adding corresponding template specializations.

roystgnr commented 7 years ago

We have a regression test for the functional case now.

Well, we have a progression test, anyway, which will become a regression test once we've actually worked all the bugs out.

dmcdougall commented 7 years ago

Do you want me to wait for you to fix the conflict before I start giving feedback?

roystgnr commented 7 years ago

Not just that conflict, but also let's get the Wy fix in, and then that ought to be the last rebase I need to do and we'll actually have some stable commits to attach comments to.

roystgnr commented 7 years ago

Can anyone replicate the regression test failure? It works fine for me, naturally...

roystgnr commented 7 years ago

But the new unit tests I'm working on don't work, so we'll fix up those bugs first and see what happens.

roystgnr commented 7 years ago

Wait a minute! "make check" is passing on Travis; it's "make distcheck" that's failing. Let's see if I can replicate that. I'm probably just forgetting to install some config file or script file.

roystgnr commented 7 years ago

OH MY GOD ALL THE CHECKS PASS MERGE IT MERGE IT MERGE IT NOW!

But seriously, @dmcdougall, I think we're good to go. Is there anything left I should change first?