search

epfl-ecps / channelflow

Channelflow is a software system for numerical analysis of the incompressible fluid flow in channel geometries, written in C++ and MPI-parallelized.
GNU General Public License v2.0
67 stars 29 forks source link

NaN default for a,b #14

Closed johnfgibson closed 4 years ago

johnfgibson commented 5 years ago

Change default for a,b arguments from 0 to NaN. Previously, the check on the default value prevented changing either a or b to zero.

codecov-io commented 5 years ago

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #14      +/-   ##
==========================================
- Coverage    27.2%   26.95%   -0.25%     
==========================================
  Files          63       63              
  Lines       15328    15328              
  Branches     7253     7254       +1     
==========================================
- Hits         4170     4132      -38     
- Misses      11158    11159       +1     
- Partials        0       37      +37
Impacted Files Coverage Δ
channelflow/helmholtz.h 66.66% <0%> (-33.34%) :arrow_down:
channelflow/bandedtridiag.h 83.33% <0%> (-16.67%) :arrow_down:
channelflow/flowfield.h 73.87% <0%> (-15.32%) :arrow_down:
cfbasics/cfvector.h 89.28% <0%> (-5.36%) :arrow_down:
cfbasics/cfarray.h 55% <0%> (-5%) :arrow_down:
cfbasics/mathdefs.h 36.06% <0%> (-4.92%) :arrow_down:
cfbasics/cfbasics.h 18.88% <0%> (-2.79%) :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 99e1dab...9ce1b8c. Read the comment docs.

johnfgibson commented 5 years ago

I don't see how the reduced test coverage reported by codecov is accurate. I changed the default value of two parameters in tools/changegrid.cpp. This should have no effect whatsoever on test coverage.

reetzelhaft commented 5 years ago

Hi John, I see why you did the modifications. Doesn't the same logic apply to Lx and Lz? I never used 2D FlowFields...

About the unexpected loss in coverage. According to Codecov this might come from failing CI tests. Since we had some issues on the Travis legacy platform travis.org, this might well be the explanation. I asked travis to move us to travis.com now. Hopefully it works now.

sajjadazimi commented 5 years ago

Unlike a and b, for Lx and Lz a zero value is meaningless, and the user should never set these variables to zero.