Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Adaptive explicit extrapolation #714
Adaptive explicit extrapolation #714
Changes from 14 commits
b447213
a2f6adb
2990b2c
2b51658
78c8bdb
68fbad8
a49313b
3374c35
257ab84
5a1c2d9
e7ceaaa
781ac67
4d48247
3fb696b
36be1d9
56150bc
a2e15d6
e7f9383
0012a27
3bb27e5
0fb345f
0d51a01
24c8cd3
3a2a4a9
d99152f
9b3ed55
aab7438
2b4626f
ed420fb
e080e64
ec087d7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an issue with PI adaptivity here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these need to be Rational? Maybe it doesn't matter all that much, but Bigs are allocating, so it would be worthwhile to profile if the calculations on these actually end up being costly, and whether just having them as Float64 may be numerically stable enough to get the job done. If Float64 is enough, then typing these to make
t
oru
would make sense.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't played with Floats here so far.
The good thing about the
BigInt
s is that the weights are converted inperform_step
to the type ofu
. That allows to adapt the precision of the problem.I'd suspect that if one uses
Float64
weights and say aBigFloat
representation for the problem, the approximation error will not get any better thaneps(Float64)
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about just converting to
u
's type here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see - the caches are recreated for every call of
solve
, right? Yes, then the type ofu
should be an option.I guess I will set up an experiment to check the stability of the weights!
BTW: Would it be possible to move the weights into the algorithm's
struct
? That way the computation of the tables for everysolve
call would be bypassed..There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
peculiar that the type of this is needed. I'll see where this goes...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the amount of pre-computations is based on
n_max
, it actually matters in small enough settings. That is something that may be worth remembering to document, or at least test a little bit. BTW, at whenn_max
does overflow start to occur in the sequences?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I will mention this in the documentation.
As far as I've tested the constructor, I have not been able to produce an overflow (for any sequence) if using
BigInt
. As I replacedBigInt
byInt
, I got an overflow for the Romberg sequence andn_max = 6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it might be easier to just stick the
cc
in here, using composition to improve code re-use. But again with the other comment, optimizing code simplicity and re-use can come in a later PR.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I decided against that to make the call
@unpack subdividing_sequence = cache
and such possible for both caches.Would you prefer an
if - else
statement before calling@unpack
in order to check if a cache is mutable or not?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like some refactoring can make this re-use a lot of code from the previous cache function. That's a second order issue that we can handle later though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's true. Am I "allowed" to define a function producing the weights etc. somewhere? If so where?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just do it in another part of this file, and then call it from in here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok I will change that!