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

Add DDETST problems and restructure existing DDE problems #39

Merged
merged 1 commit into from
Jun 30, 2019
Merged

Add DDETST problems and restructure existing DDE problems #39

merged 1 commit into from
Jun 30, 2019

Conversation

devmotion
Copy link
Member

I tried to add the currently missing DDE problems from DDETST (and thereby fix #11). I also tried to organize the existing DDE problems better and replace build_prob_dde_* functions with remake. I added deprecations for backwards compatibility, but I guess if the changes in this PR seem acceptable one could also just update the DelayDiffEq test suite and not include those deprecations.

I haven't tested all DDETST problems yet but it seems that especially the ones with vanishing delay are challenging for DelayDiffEq at the moment, so, e.g., trying to solve the naively implemented problem prob_dde_DDETST_F1 with vanishing delay at the initial time point yields an error due to interpolation problems in the history function. I have to investigate this more closely but my initial guess is that currently DelayDiffEq can not deal with the derivative at the initial time point being specified only via the history function (and not as an additional variable in the state vector).

Generally, while implementing the test suite I made the following observations:

  • It seems a bit unintuitive that one has to provide both a history function and an initial state - the initial state should just be the history function evaluated at the initial time point. I think it would be nice to at least be able to specify only a history function. Related to this, the current logic for guessing the order of the discontinuity at the initial time point seems strange - checking u0 == h(p, t0) should always be true. I think we should just get rid of this check and by default assume that the function is discontinuous at the initial time point. Since the order of the discontinuity is an immediate consequence of u0 and h and not dependent on any numerical algorithm, it would be reasonable to include it in DDEProblem, I guess.

  • It's a bit unfortunate that the type of u is not accessible from inside the user-implemented history function. This makes it impossible for users to write a generic history function that works with arbitrary types of u and t - in particular, if number types with units are used this is a problem. I added a hack in the implementation of the standard problems with constant delays by setting p = typeof(eltype(u)), but this seems unsatisfying. Maybe this could be solved by reviving the old idea of passing integrator instead of p to both f and h?

@ChrisRackauckas ChrisRackauckas merged commit 1cb79cd into SciML:master Jun 30, 2019
@ChrisRackauckas
Copy link
Member

Thanks! This is very thorough. Now we just need some way to automatically benchmark a bunch of stuff from a test algorithm.

@devmotion
Copy link
Member Author

I guess it'd be good to update and extend DiffEqBenchmarks. Can you register a new release of DiffEqProblemLibrary? Then I can update DelayDiffEq as well.

By the way, another thing I noticed is that f_analytic(u0, p, t) doesn't make much sense - I think it should rather depend on the history function (or at least on the history function as well).

@ChrisRackauckas
Copy link
Member

It probably should. Analytic overloads are pretty safe to break, so go ahead with whatever's needed there. For the history function, we should have a common arg for using integrator instead of p, and then we just need to make every package handle that well.

@ChrisRackauckas
Copy link
Member

Along with using integrator instead of p, it would make sense to make sure every integrator has userdata as a cache.

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.

DDETST Equations
2 participants