Closed JohanMabille closed 4 years ago
I think we can merge this once the SWEs tests pass. It looks like they're still failing.
The failure is due to arguments types mismatch: a double is expected on the C++ side while an int is passed from the Python. Before using the dictionaries, the conversion was automatic and everything worked fine. Now the parameter ends up in the wrong scalar dictionary. When asking for this scalar in C++, since it is not found in the map, a default value is returned.
We need to catch these bugs quickly, I will improve the dictionaries for scalar in a dedicated PR and rebase all the failing PRs on it once it is merged (the other failures are probably due to the same issue).
See #1157
Merging #1152 into master will increase coverage by
0.27%
. The diff coverage is67.13%
.
@@ Coverage Diff @@
## master #1152 +/- ##
==========================================
+ Coverage 51.63% 51.90% +0.27%
==========================================
Files 539 540 +1
Lines 102272 103687 +1415
==========================================
+ Hits 52805 53816 +1011
- Misses 49467 49871 +404
Impacted Files | Coverage Δ | |
---|---|---|
proteus/WaveTools.py | 0.00% <0.00%> (ø) |
|
proteus/config/copper.py | 0.00% <ø> (ø) |
|
proteus/mprans/RANS3PSed.py | 0.00% <0.00%> (ø) |
|
proteus/mprans/VOS3P.py | 0.00% <0.00%> (ø) |
|
proteus/mprans/Dissipation.py | 11.87% <12.50%> (+0.41%) |
:arrow_up: |
proteus/mprans/Kappa.py | 11.73% <12.50%> (+0.27%) |
:arrow_up: |
...sts/solver_tests_mprans/import_modules/cavity2d.py | 66.18% <25.00%> (ø) |
|
proteus/tests/griffiths_lane_6/sm_gl_6_3d_n.py | 94.73% <50.00%> (-5.27%) |
:arrow_down: |
proteus/tests/griffiths_lane_6/sm_gl_6_3d_p.py | 71.87% <50.00%> (-0.74%) |
:arrow_down: |
proteus/tests/TwoPhaseFlow/damBreak_PUMI.py | 92.59% <92.59%> (ø) |
|
... and 13 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 66b6701...a0221dc. Read the comment docs.
@ejtovar I think this one is ready for a merge.
Thanks @JohanMabille!
Mandatory Checklist
Please ensure that the following criteria are met:
As a general rule of thumb, try to follow PEP8 guidelines.
Description