[GiNaC-list] [PATCH] fix bug/feature causing non-deterministic output

[Valery Yundin]

Hi,

On 29 May 2014 07:31, Alexei Sheplyakov <alexei.sheplyakov at gmail.com> wrote:
> The output of GiNaC is reproducible: the same calculation gives equal
> expressions, it's just the term order which might differ.
Different term order may affect some algorithms and result in
completely different expressions (like solving undetermined system).

> ( echo 'expand(('; cat file1; echo ') - ('; cat file2 '));' ) | ginsh
To use this users need to write every GiNaC expression in a separate file?

> I'm terribly sorry, but everything which makes the hash function worse
> (slower or produces more collisions) is not acceptable.
And I'm terribly sorry that you care about a single function more than
about the whole library.

> Unfortunately this version is even worse than the previous one.
> 1) Now everyone have to copy-paste gethashseed() into their classes.
They don't have to, only if they want deterministic ordering.

> 2) Previously the hash seed was computed by a simple (inline) function.
>    Now the calculation involves a call of the virtual method (which is
>    *much* slower).
Do you care about hash function being abstractly fast or the speed of
the GiNaC as a whole? If first then I have bad news for you, GiNaC has
*hundreds* of virtual functions all of which are "much slower". If you
ever ran a profiler on GiNaC you'd know that hashing is nowhere near
being a hotspot.

Did you happen to compare them on a real world example and seen a
visible difference? Otherwise you are refusing a patch (which changes
user experience by adding determinism) using an argument based on a an
abstract speed of a single function (which does not affect user
experience in any visible way).

> If you don't mind making your computations 10% slower you can just
> unconditionally
Again, where does this 10% come from? It is not even close to 1%.

Sincerely,
Valery Yundin