Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix dp errors #153

Merged
merged 8 commits into from Oct 2, 2019
Merged

Fix dp errors #153

merged 8 commits into from Oct 2, 2019

Conversation

TorkelE
Copy link
Member

@TorkelE TorkelE commented Sep 27, 2019

While preparing a short tutorial for making bifurcation diagrams I actually discovered an error where some paths were not displayed correctly. It was basically due to be mixing up some parameters in the heuristics. Should be sorted now.

@TorkelE
Copy link
Member Author

TorkelE commented Oct 1, 2019

Okay, I've investigated this error quite closely. Basically it works fine on Julia 1.1 and above, but errors on Julia 1.0. The reasons for this seems to sit quite deep, and partially due to behaviours in HomotopyContinuation.jl. It is likely possible to make it work, but will require a significant time investment, and some additional code which is unnecessary on newer julia versions.

Either we let this PR rest until we make DiffEqBiological require a later Julia version (1.1). However, I do think this change is somewhat important for bifurcation analysis, so the rest time should not get too long.

Or I make some slight modification to the test, making them a bit less strong, so the 1.0 errors slip by (they are not that serious. Basically some paths are added twice to the diagram. However, when they are plotted the identical paths overlap and the plot itself is not different. There might be a problem if someone uses the bifurcation diagram objects for non-visual purposes)

@isaacsas
Copy link
Member

isaacsas commented Oct 1, 2019

I'm fine with requiring 1.1 and up, but I think @ChrisRackauckas was having problems somewhere else that had required us to still support 1.0. Maybe he can chime in.

We could make a major version number increment to indicate 1.1 is now required.

@ChrisRackauckas
Copy link
Member

It blocks the DifferentialEquations.jl tests. Maybe put an if statement on the bifurcation tests and only run them if >= v"1.1" and otherwise throw a warning that those are disabled on 1.0?

@TorkelE
Copy link
Member Author

TorkelE commented Oct 1, 2019

It blocks the DifferentialEquations.jl tests. Maybe put an if statement on the bifurcation tests and only run them if >= v"1.1" and otherwise throw a warning that those are disabled on 1.0?

That's sounds like a good solution.

@codecov
Copy link

codecov bot commented Oct 1, 2019

Codecov Report

Merging #153 into master will increase coverage by 0.11%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #153      +/-   ##
==========================================
+ Coverage   93.65%   93.77%   +0.11%     
==========================================
  Files           9        9              
  Lines         962      964       +2     
==========================================
+ Hits          901      904       +3     
+ Misses         61       60       -1
Impacted Files Coverage Δ
src/equilibrate_utils.jl 98.39% <100%> (+0.43%) ⬆️
src/plot_recipes.jl 97.91% <0%> (-0.09%) ⬇️

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 9ba853c...70b6021. Read the comment docs.

TorkelE and others added 4 commits October 1, 2019 16:06
Co-Authored-By: Christopher Rackauckas <Contact@ChrisRackauckas.com>
Co-Authored-By: Christopher Rackauckas <Contact@ChrisRackauckas.com>
Co-Authored-By: Christopher Rackauckas <Contact@ChrisRackauckas.com>
Co-Authored-By: Christopher Rackauckas <Contact@ChrisRackauckas.com>
@coveralls
Copy link

coveralls commented Oct 1, 2019

Coverage Status

Coverage decreased (-0.9%) to 91.876% when pulling 70b6021 on Fix_dp_errors into 9ba853c on master.

@ChrisRackauckas ChrisRackauckas merged commit 2bc171d into master Oct 2, 2019
@ChrisRackauckas ChrisRackauckas deleted the Fix_dp_errors branch October 2, 2019 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants