simon-git: spigot (master): Simon Tatham

Tue Mar 20 18:57:22 GMT 2018

TL;DR:
  105df4e Fix segfault in Core2 greeting diagnostic.
  51fb780 Put wrapper functions around Source::gen_{interval,matrix}.
  2f28dff Introduce an internal-fault reporting mechanism.
  5cd4c38 Check that Sources reliably refine their starting intervals.
  fb664ca Add lots of missing dgreet calls.
  82f84bd Fix duplicate call to RationalSource::get_matrix.
  12e2c2c Add initial forcing segment in several Source classes.
  66aa8dc Fix two separate non-refiningness bugs in HgRational.
  163755f Remove all remaining 'I totally made this up' comments.
  716c828 Update the TODO list.

Repository:     https://git.tartarus.org/simon/spigot.git
On the web:     https://git.tartarus.org/?p=simon/spigot.git
Branch updated: master
Committer:      Simon Tatham <anakin at pobox.com>
Date:           2018-03-20 18:57:22

commit 105df4efb4967fd4c6d4d1c9387b2a66030d8396
web diff https://git.tartarus.org/?p=simon/spigot.git;a=commitdiff;h=105df4efb4967fd4c6d4d1c9387b2a66030d8396;hp=17c4cea8616fee0673c164db5ef67f0149959437
Author: Simon Tatham <anakin at pobox.com>
Date:   Mon Mar 19 19:09:47 2018 +0000

    Fix segfault in Core2 greeting diagnostic.

    When Core2 is instantiated with both inputs logically the same (for
    spigot_square, spigot_quadratic, etc), this is actually represented by
    setting sx to be the common input and sy to be nullptr, because
    obviously you can't point both unique_ptrs at the same object.

    Therefore, it's a mistake to unconditionally dereference sy in the
    debug greeting message. Ahem.

 arithmetic.cpp | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

commit 51fb780f85e91eee648126fefbae7fb6eb79a2bd
web diff https://git.tartarus.org/?p=simon/spigot.git;a=commitdiff;h=51fb780f85e91eee648126fefbae7fb6eb79a2bd;hp=105df4efb4967fd4c6d4d1c9387b2a66030d8396
Author: Simon Tatham <anakin at pobox.com>
Date:   Mon Mar 19 19:19:17 2018 +0000

    Put wrapper functions around Source::gen_{interval,matrix}.

    The wrappers (called, fairly arbitrarily, get_* rather than gen_*) are
    public, non-virtual, and implemented centrally in the Source class
    itself, which makes them a good place to put code that needs to be run
    uniformly for all kinds of Source no matter what kind of Core is going
    to use it.

    NFC: at the moment the wrapper functions don't actually do anything.
    This just sets them up and ensures the rest of the code base is
    calling the right one of the virtual and non-virtual versions of the
    function.

    (I've made the virtual functions protected, to avoid Cores
    accidentally calling them. I'd have liked to declare the wrappers as
    'final' to avoid subclasses accidentally overriding them - but you
    aren't allowed to declare a function as final unless you also make it
    virtual, which seems exactly anti-helpful to me - precisely _because_
    I never want it overridden, I'd rather avoid the overhead of having to
    make it virtual!)

 arithmetic.cpp |  8 ++++----
 spigot.cpp     | 18 ++++++++++++++++--
 spigot.h       |  3 +++
 unary.cpp      |  4 ++--
 4 files changed, 25 insertions(+), 8 deletions(-)

commit 2f28dffde9551aed4209a224c08305364c846224
web diff https://git.tartarus.org/?p=simon/spigot.git;a=commitdiff;h=2f28dffde9551aed4209a224c08305364c846224;hp=51fb780f85e91eee648126fefbae7fb6eb79a2bd
Author: Simon Tatham <anakin at pobox.com>
Date:   Tue Mar 20 18:42:02 2018 +0000

    Introduce an internal-fault reporting mechanism.

    internal_error is (yet) another subclass of the spigot_error exception
    class. It comes with a constructor which helpfully prefixes a scary
    "INTERNAL FAULT" message.

    It also comes with a pre-cooked method that throws one, inherited by
    any class deriving from Debuggable, which automatically incorporates
    into the error message the debug id of the object that caused the
    fault. So if I get one of these messages, I can immediately re-run
    with --debug=<that id>, and see all the rest of the diagnostic
    messages from the same object.

    As yet I haven't used this new system for anything, but I'm about to,
    and in future perhaps I should migrate some of the existing assertions
    to it.

 error.h  | 19 +++++++++++++++++++
 misc.cpp | 14 ++++++++++++--
 spigot.h |  6 ++++++
 3 files changed, 37 insertions(+), 2 deletions(-)

commit 5cd4c38d5fc2b6a5d2b7170bfa8f959b346ccb6d
web diff https://git.tartarus.org/?p=simon/spigot.git;a=commitdiff;h=5cd4c38d5fc2b6a5d2b7170bfa8f959b346ccb6d;hp=2f28dffde9551aed4209a224c08305364c846224
Author: Simon Tatham <anakin at pobox.com>
Date:   Tue Mar 20 18:41:51 2018 +0000

    Check that Sources reliably refine their starting intervals.

    This has been a long-standing TODO list entry for ages, and after I
    had so much trouble figuring out how to choose the starting interval
    in the recent hypergeometric function class, I decided its time had
    come. So this change fills in the new Source wrapper functions with
    code that stashes a copy of the starting interval, and for every
    sequence of matrices returned from get_matrix() ending in one with
    force=false, checks that the product of that sequence really does
    reduce the starting interval to a subinterval of itself. If not, an
    internal_error exception is thrown.

    This should mean that if a Source returns a self-inconsistent data
    stream, it's detected as early as possible, and (once you re-run with
    debugging logs enabled) gives a useful error message close to the
    point of failure rather than generating incorrect digits or an
    assertion failure a long time later in the computation.

    Unfortunately, it causes a lot of the test suite to fail with just
    such internal errors - which proves it was worth doing! Next step: fix
    all those failures.

 TODO.txt   |  24 --------
 spigot.cpp | 203 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 spigot.h   |   4 ++
 3 files changed, 205 insertions(+), 26 deletions(-)

commit fb664cae1640314b95c78ebe3bc6d7989b3dabce
web diff https://git.tartarus.org/?p=simon/spigot.git;a=commitdiff;h=fb664cae1640314b95c78ebe3bc6d7989b3dabce;hp=5cd4c38d5fc2b6a5d2b7170bfa8f959b346ccb6d
Author: Simon Tatham <anakin at pobox.com>
Date:   Mon Mar 19 19:12:06 2018 +0000

    Add lots of missing dgreet calls.

    I _think_ there should now be one in every subclass of Source, which
    means that any class for which the new refinement check can report a
    diagnostic should also come with a greeting telling you what it
    actually was.

 algebraic.cpp |  1 +
 consts.cpp    | 41 +++++++++++++++++++++++++++++++++++++++++
 erf.cpp       |  1 +
 exp.cpp       |  2 ++
 expint.cpp    |  4 ++++
 gamma.cpp     |  3 +++
 hypergeom.cpp |  2 ++
 io.cpp        |  4 ++++
 monotone.cpp  |  2 ++
 sqrt.cpp      |  1 +
 trig.cpp      |  4 ++++
 unary.cpp     |  1 +
 12 files changed, 66 insertions(+)

commit 82f84bd37392a97e78f378e03c16d047c21e2e20
web diff https://git.tartarus.org/?p=simon/spigot.git;a=commitdiff;h=82f84bd37392a97e78f378e03c16d047c21e2e20;hp=fb664cae1640314b95c78ebe3bc6d7989b3dabce
Author: Simon Tatham <anakin at pobox.com>
Date:   Mon Mar 19 19:12:25 2018 +0000

    Fix duplicate call to RationalSource::get_matrix.

    The new refinement check threw an error in what should have been the
    simplest number source of all, because Generator was managing to try
    to extract _two_ matrices from its get_matrix() function, which should
    never have happened because the first one represented a constant
    function, and once that's happened, there shouldn't be any need to
    request further information from the Source.

    The problem was the direct call to core->refine() in
    Generator::ensure_started(), which should have gone through the
    followup code that checks for a constant function and sets the
    'we_are_now_constant' flag. Easily fixed.

 spigot.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

commit 12e2c2c47b9590455f90e5764ec051836b96b242
web diff https://git.tartarus.org/?p=simon/spigot.git;a=commitdiff;h=12e2c2c47b9590455f90e5764ec051836b96b242;hp=82f84bd37392a97e78f378e03c16d047c21e2e20
Author: Simon Tatham <anakin at pobox.com>
Date:   Mon Mar 19 19:10:37 2018 +0000

    Add initial forcing segment in several Source classes.

    The new refinement check turned up failures in ErfBaseRational,
    LowerGammaBaseRational, UpperGammaBaseRational, SinRational,
    CosSqrtRational, AtanRational and TrigIntSeries which can all be fixed
    by the same basic technique, namely to identify a point in the stream
    of output matrices after which we can rely on everything having the
    right sign and/or inter-term ratio to make the matrices refining, and
    simply return force=true for everything before that point.

    The most interesting one of these was LowerGammaBaseRational, for
    which even _after_ the safe point we still have to return force=true
    for some matrices, specifically, in a pattern TTTF TTTF TTTF to group
    the steps of the continued fraction into fours. (This is because of
    the pattern of signs in the non-canonical continued fraction
    representation we're using.)

    The _least_ interesting one was AtanRational, in which it's only the
    very first anomalous matrix that's potentially problematic at all!

 erf.cpp     | 18 +++++++++++++-----
 gamma.cpp   | 41 ++++++++++++++++++++++++++++++++---------
 trig.cpp    | 56 +++++++++++++++++++++++++++++++++++++++++++-------------
 trigint.cpp | 14 ++++++++++++--
 4 files changed, 100 insertions(+), 29 deletions(-)

commit 66aa8dcfe62b314e0374612c065427659dd90c7a
web diff https://git.tartarus.org/?p=simon/spigot.git;a=commitdiff;h=66aa8dcfe62b314e0374612c065427659dd90c7a;hp=12e2c2c47b9590455f90e5764ec051836b96b242
Author: Simon Tatham <anakin at pobox.com>
Date:   Tue Mar 20 06:55:18 2018 +0000

    Fix two separate non-refiningness bugs in HgRational.

    This was the class whose interval / refinement analysis was so
    complicated as to motivate me to finally write the refining checker -
    and rightly so, it turns out, because even with all the analysis I
    could do up front there were still two bugs in the resulting code.

    Fortunately, neither was too hard. One was a missing abs() (we should
    have checked that the _absolute_ value of the inter-term ratio was
    below its limit, rather than accidentally letting through arbitrarily
    enormous negative ratios), and the other was that having carefully
    calculated a threshold ratio mn/md different from the limiting ratio
    rn/rd I then went and computed the starting interval size based on the
    latter instead of the former. Ahem.

 hypergeom.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

commit 163755f92c524544b2de34eb136a489a77f1c285
web diff https://git.tartarus.org/?p=simon/spigot.git;a=commitdiff;h=163755f92c524544b2de34eb136a489a77f1c285;hp=66aa8dcfe62b314e0374612c065427659dd90c7a
Author: Simon Tatham <anakin at pobox.com>
Date:   Tue Mar 20 07:18:32 2018 +0000

    Remove all remaining 'I totally made this up' comments.

    Long ago I wrote these comments in several gen_interval functions,
    when I wasn't really sure what I _should_ be writing in as the
    starting interval for a given Source class and just empirically
    stopped at the first thing that seemed to be delivering the right
    answers. So they were intended to carry a subtext of 'if bugs are ever
    found in this part of the code, it won't be a great surprise'.

    I removed several of the comments in the course of fixing some of the
    classes in question to pass the new refinement check, on the basis
    that now I actually understood what was going on in those classes. Now
    I think it's reasonable to also remove the corresponding comments in
    the classes I _didn't_ have to fix, on the grounds that the refinement
    check has reassured me that I didn't get the values wrong.

 exp.cpp | 2 --
 1 file changed, 2 deletions(-)

commit 716c828ae4e2e5da10af3c28c5df2bfd07ebb7fd
web diff https://git.tartarus.org/?p=simon/spigot.git;a=commitdiff;h=716c828ae4e2e5da10af3c28c5df2bfd07ebb7fd;hp=163755f92c524544b2de34eb136a489a77f1c285
Author: Simon Tatham <anakin at pobox.com>
Date:   Tue Mar 20 07:15:14 2018 +0000

    Update the TODO list.

    I've decided that experimenting with the Imperial College chained-
    Core2 technique for evaluating a series at an irrational is just too
    blue-sky for me to ever realistically expect to get round to, so I'm
    deleting it from the checked-in TODO list. (If I ever feel like trying
    it anyway - which Murphy's Law predicts is now _more_ likely - then I
    can always recover my thoughts about it from source control.)

    Also, I've added a writeup of another idea that I've been turning over
    in my head for a while now: a spigot-based graph plotting utility.
    Unfortunately this needs a piece of substantial preliminary work, but
    that work would help the interactive REPL idea too, so I've written
    that up as a separate idea.

    (Finally, moved the inotify-based input-file wait idea to the end of
    the list. I can see it might be useful in principle, but it's not
    really high priority for me.)

 TODO.txt | 152 ++++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 83 insertions(+), 69 deletions(-)