davidbrochart commented 4 years ago

Mandatory Checklist

Please ensure that the following criteria are met:

[x] Title of pull request describes the changes/features
[x] Request at least 2 reviewers
[x] If new files are being added, the files are no larger than 100kB. Post the file sizes.
[x] Code coverage did not decrease. If this is a bug fix, a test should cover that bug fix. If a new feature is added, a test should be made to cover that feature.
[x] New features have appropriate documentation strings (readable by sphinx)
[x] Contributor has read and agreed with CONTRIBUTING.md and has added themselves to CONTRIBUTORS.md

As a general rule of thumb, try to follow PEP8 guidelines.

Description

codecov[bot] commented 4 years ago

Codecov Report

Merging #1147 into master will decrease coverage by 0.00%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1147      +/-   ##
==========================================
- Coverage   51.77%   51.76%   -0.01%     
==========================================
  Files         540      540              
  Lines      103400   103400              
==========================================
- Hits        53531    53528       -3     
- Misses      49869    49872       +3

Impacted Files	Coverage Δ
proteus/tests/test_boundaryconditions.py	`97.20% <0.00%> (-0.37%)`	:arrow_down:

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 b2bc723...ca7b23e. Read the comment docs.

cekees commented 4 years ago

HI @davidbrochart. Seems like something is wrong here--116 files changed?

ejtovar commented 4 years ago

@cekees it looks like the files that were changed were auxiliary files that can be produced from the tests such as mesh files, etc. @zhang-alvin and I discussed this the other day that these were probably accidentally added to the repo via a .gitignore or .gitattribute file change.

davidbrochart commented 4 years ago

@cekees I can't get rid of these files, they are related to LFS but even when I delete my GitHub repo and fork proteus again, they're back and seen as modified. I don't know where it comes from, my git settings seem okay. Anyway I don't think this is the issue here. pytest silently stops on Travis, so it's hard to debug. I will try to have access to a Mac. @ejtovar do you get the same behavior when you run the tests on your Mac?

ejtovar commented 4 years ago

@cekees In this particular commit of yours https://github.com/erdc/proteus/commit/0955291f2a03bab4b77d9fbbe4a0312efccdc76d, you added 119 files and I'm guessing these are the 116 files David is removing.

@davidbrochart When I do make test I get the "No tests ran" error. Chris and I discussed this awhile back. I was able to get some particular tests to actually do something, but I haven't had a chance to look at it again in detail.

cekees commented 4 years ago

Ugh. Thanks @ejtovar. Not sure how we missed that./cc @zhang-alvin

zhang-alvin commented 4 years ago

@cekees, that was my bad since I reviewed the PR. Thankfully, those files should be small since they survived the repo cleanup. We can either untrack them from git lfs or just track them again with git lfs and the impact on the memory footprint should be small.

cekees commented 4 years ago

@ejtovar what happens when you just run pytest from the command line using the command we use in .travis.yaml?

ejtovar commented 4 years ago

I am attaching the output here: pytest_output.txt. It's the same 'No tests ran' message.

cekees commented 4 years ago

I am attaching the output here: pytest_output.txt. It's the same 'No tests ran' message.

Maybe try without the codecov option, and try going into proteus/tests and running from inside there (individual tests are letting it find test recursively).

ejtovar commented 4 years ago

Doing

py.test -n 1 --dist=no --forked -v . --ignore ./POD  --ignore ./MeshAdaptPUMI

inside /proteus/tests gives the same "No tests ran" error, but going into a specific directory like poisson_2d and running

 py.test -n 1 --dist=no --forked -v .

actually runs the tests.

cekees commented 4 years ago

@davidbrochart I gave @ejtovar some things to try on his machine. Most of the tests run, though there is a seg fault that needs fixing and some tests that are sensitive to round off that need improving. However, neither the make test command nor the py.test command it generates works on os x. Any ideas on that?

davidbrochart commented 4 years ago

No idea, I still haven't got access to a Mac machine.

davidbrochart commented 4 years ago

I ran with pytest -s: https://travis-ci.com/github/erdc/proteus/jobs/324489386

[0]PETSC ERROR: ------------------------------------------------------------------------
[0]PETSC ERROR: Caught signal number 4 Illegal instruction: Likely due to memory corruption
[0]PETSC ERROR: Try option -start_in_debugger or -on_error_attach_debugger
[0]PETSC ERROR: or see https://www.mcs.anl.gov/petsc/documentation/faq.html#valgrind
[0]PETSC ERROR: or try http://valgrind.org on GNU/linux and Apple Mac OS X to find memory corruption errors
[0]PETSC ERROR: configure using --with-debugging=yes, recompile, link, and run 
[0]PETSC ERROR: to get more information on the crash.
application called MPI_Abort(MPI_COMM_WORLD, 59) - process 0
[unset]: write_line error; fd=-1 buf=:cmd=abort exitcode=59
:
system msg for write_line failure : Bad file descriptor

cekees commented 4 years ago

I ran with pytest -s: https://travis-ci.com/github/erdc/proteus/jobs/324489386

[0]PETSC ERROR: ------------------------------------------------------------------------
[0]PETSC ERROR: Caught signal number 4 Illegal instruction: Likely due to memory corruption
[0]PETSC ERROR: Try option -start_in_debugger or -on_error_attach_debugger
[0]PETSC ERROR: or see https://www.mcs.anl.gov/petsc/documentation/faq.html#valgrind
[0]PETSC ERROR: or try http://valgrind.org on GNU/linux and Apple Mac OS X to find memory corruption errors
[0]PETSC ERROR: configure using --with-debugging=yes, recompile, link, and run 
[0]PETSC ERROR: to get more information on the crash.
application called MPI_Abort(MPI_COMM_WORLD, 59) - process 0
[unset]: write_line error; fd=-1 buf=:cmd=abort exitcode=59
:
system msg for write_line failure : Bad file descriptor

We haven't upgraded to petsc 3.12 yet--still on 3.11.3.

davidbrochart commented 4 years ago

Do you mean this is wrong: https://github.com/erdc/proteus/blob/master/environment-dev.yml#L20

cekees commented 4 years ago

Do you mean this is wrong: https://github.com/erdc/proteus/blob/master/environment-dev.yml#L20

Possibly. The versions of petsc that have actually been heavily used and tested on HPC systems are here: https://github.com/erdc/stack/blob/master/pkgs/petsc/petsc.yaml

I'm still not actually using the conda environment much, so maybe it's working fine for others. /cc @tridelat @zhang-alvin @ejtovar

davidbrochart commented 4 years ago

The feedstock also has petsc 3.12: https://github.com/conda-forge/proteus-feedstock/blob/master/recipe/meta.yaml#L41 Actually trying to install petsc 3.11.3 locally is not possible currently, the environment is not solvable. If you think this is worth it we can try and have a solvable environment for 3.11.3. But given that it works fine on Linux, I'm not sure it's the cause of the issue on OSX.

cekees commented 4 years ago

OK, @ejtovar is looking at some different seg faults on the osx side. petsc may not be the origin of the problem, it's just catching the error. I'll see about upgrading my install to a newer version of petsc.

cekees commented 4 years ago

addressed by #1178

erdc / proteus

Osx tests #1147

Mandatory Checklist

Description

Codecov Report