Skip to content
This repository has been archived by the owner on Sep 9, 2024. It is now read-only.

WIP: Make coefficients Rational for better generics #60

Closed
wants to merge 1 commit into from

Conversation

tomasaschan
Copy link
Contributor

This is still just WIP, but I need some advice to keep this moving.

@pwl mentioned that it might be a good idea to convert all the coefficients in this code to rational, to make generic programming easier. I agree, since Float64 is quite poisonous in this aspect; the promotion of almost anything and a Float64 will be a Float64. We've discussed before how to handle inputs of types that don't make sense in an ODE context and agreed that converting to floats is the most rational (heh...) thing to do, but there still might, for example, be applications for Float32 arguments, or arguments of some user-defined number type, where we don't want to force Float64s on the user.

The PR currently only converts the coefficient tables that were already specified as fractions, but there is one table that wasn't (and I couldn't find a reference where it was) and there's also a bunch of coefficients and constants scattered throughout the various solvers, which are all specified as Float64 literals. Do we want to try to make something more sensible with those? What?

It could also be argued that making the code generic yet type stable is too complicated to be worth the effort, in which case it's a good thing that I didn't spend more than a few minutes on this PR yet. (But I'd hate to end the discussion about this with "a Float64 ought to be enough for anyone"...)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.97%) to 96.12% when pulling 5899698 on generic-constants into 4a71fa0 on master.

@tomasaschan
Copy link
Contributor Author

(The build failures are because Coveralls doesn't work on 0.4 - all our tests pass on both versions...)

@mauro3
Copy link
Contributor

mauro3 commented Mar 26, 2015

The original paper by Kaps and Rentrop (1979) doesn't give ratios for the GRK4T coefficients, so presumably they are not know/don't exist.

@stevengj
Copy link
Contributor

One problem is that conversion of constant rational coefficients to floating-point cannot be done at compile time, so we have to be cautious about using these in inner loops.

@mauro3
Copy link
Contributor

mauro3 commented Mar 26, 2015

Could/should MathConst be used instead?

Probably better convert the table before the inner loops to the used float-type.

@mauro3
Copy link
Contributor

mauro3 commented May 29, 2015

I think this has been addressed by #68, right?

@tomasaschan
Copy link
Contributor Author

So it would seem.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants