Adding APM solver to the public version #116
Conversation
|
Hi @jcpassy , thank you for submitting this code. As you may see, both the code test suite and compilation options tests are failing (look at the CircleCI box above). We can't accept the PR until you sort that out. Could you please look at the outputs of the failed tests, fix the problems, and update your PR? |
|
@jcpassy , just following up on this! |
|
Hi @bwoshea , sorry I was packed at work lately. I will do that over this coming weekend! :) |
|
@jcpassy not a problem - thanks for letting us know about your plans! |
jcpassy
force-pushed
the
issues/apm-solver
branch
3 times, most recently
from
516fb5c to
56a437d
Compare
|
@bwoshea this is green now but I still need to make the tests visible by the test_suite. Should be done soon. |
jcpassy
force-pushed
the
issues/apm-solver
branch
from
0ffc68d to
f1ae618
Compare
|
@bwoshea @gregbryan My work is done here I think and the branch is green. :) |
|
@jcpassy Agreed, all tests pass. I'll inform reviewers! |
|
@gregbryan and @fearmayo , this PR is ready for review. Thank you! |
|
@jcpassy could you please update your pull request using the tip of the enzo-dev repository? We updated the gold standard for tests. |
|
We had a bug recently introduced that made the tests fail when you submitted your PR. It was fixed yesterday in #131, which also conflicts with your changes. Could you merge the latest changes from enzo-project:master so the tests can run again? Thanks! |
…the paper are reproduced. Additional: * changes to make APM tests completely isolated * tests added to the test suite * updated documentation + authors list * updated .gitignore * show log in case of error in circleci
jcpassy
force-pushed
the
issues/apm-solver
branch
from
f1ae618 to
1b12cec
Compare
|
Hi @jwise77 , @bwoshea , and @gregbryan , Cheers! |
|
Hi @jwise77 , @bwoshea, and @gregbryan, |
|
Hi All, What's happening with this PR? It looks good to me (conflicts aside). I think once the tests are passing we should accept and merge this PR in? |
|
@fearmayo Do you want to approve since it looked good to you? I'll review it, too, and we can finally merge this in! |
This is the implementation of the APM gravity solver published in Passy & Bryan (2014).