erdc / proteus

A computational methods and simulation toolkit
http://proteustoolkit.org
MIT License
88 stars 56 forks source link

Fix to allow for hotstarting with systemStepExact=False #1236

Closed zhang-alvin closed 4 years ago

zhang-alvin commented 4 years ago

Mandatory Checklist

Please ensure that the following criteria are met:

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

Description

Addresses #1235. Not setting the model time prior to proceeding with calculation leads to an infinite loop.

Thanks @cekees for the suggested fix.

Added a test within tests/TwoPhaseFlow to test for the fix. Note to keep in mind is that the test is to avoid changes to the infinite loop fix.

I tried setting a test to compare numerical results but I believe something along the lines of #1184 was causing a different behavior between the directory-level pytest call and a make test call at the root.

codecov[bot] commented 4 years ago

Codecov Report

Merging #1236 into master will decrease coverage by 0.00%. The diff coverage is 75.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1236      +/-   ##
==========================================
- Coverage   52.74%   52.74%   -0.01%     
==========================================
  Files         531      531              
  Lines      109533   109536       +3     
==========================================
  Hits        57777    57777              
- Misses      51756    51759       +3     
Impacted Files Coverage Δ
proteus/tests/TwoPhaseFlow/test_TwoPhaseFlow.py 85.48% <ø> (ø)
proteus/tests/TwoPhaseFlow/damBreak.py 89.39% <66.66%> (-1.24%) :arrow_down:
proteus/NumericalSolution.py 70.76% <100.00%> (+0.02%) :arrow_up:
proteus/tests/test_boundaryconditions.py 97.33% <0.00%> (-0.25%) :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 317373d...e0a84d2. Read the comment docs.

zhang-alvin commented 4 years ago

@cekees, I think the line in NumericalSolution is causing a massive code cov decrease. I can try moving the line elsewhere or is there some simpler fix to the drop in coverage.

I think @ejtovar ran into something similar pretty recently. I forget how it was resolved.

zhang-alvin commented 4 years ago

For reference, prior to hack/removal of test, codecov indicated -36% coverage. Removing test yields only a -0.01% drop. Going to turn the test back on to see if behavior is corrected.