I'm pretty sure that's the "major" bug we missed until now ;)
W dniu 2015-06-25 10:09, Jeroen Vermeulen napisał(a):
> Looking at replacing WordsBitmap's implementation with std::vector<bool>
> (less code, less memory) I came across this function:
>
> «
> /** check, if two hypothesis can be recombined.
> this is actually a sorting function that allows us to
> keep an ordered list of hypotheses. This makes recombination
> much quicker.
> */
> int
> Hypothesis::
> RecombineCompare(const Hypothesis &compare) const
> {
> // -1 = this < compare
> // +1 = this > compare
> // 0 = this ==compare
> int comp = m_sourceCompleted.Compare(compare.m_sourceCompleted);
> if (comp != 0)
> return comp;
>
> for (unsigned i = 0; i < m_ffStates.size(); ++i) {
> if (m_ffStates[i] == NULL || compare.m_ffStates[i] == NULL) {
> comp = m_ffStates[i] - compare.m_ffStates[i];
> } else {
> comp = m_ffStates[i]->Compare(*compare.m_ffStates[i]);
> }
> if (comp != 0) return comp;
> }
>
> return 0;
> }
> »
>
> My problem is with this conditional subtraction:
>
> «
> if (m_ffStates[i] == NULL || compare.m_ffStates[i] == NULL) {
> comp = m_ffStates[i] - compare.m_ffStates[i];
> »
>
> The result of that subtraction looks technically undefined to me, in
> which case _theoretically_ I could replace it with anything I liked
> including code to recite Homer in Morse code on the hard-disk light.
> But what is it meant to do in practice?
>
> The assignment to comp casts the value from std::ptrdiff_t to int. On a
> two's-complement system with 64-bit pointers, 32-bit ints, and a zero
> null pointer, the could would boil down to: take either m_ffStates[i] or
> ~m_ff_states[i] + 1 depending on which one is non-null, divide it by the
> size of FFState, drop the most-significant 32 bits, and compare the
> least-signficant 32 bits to zero. Occasionally you may get a completely
> unexpected zero even though one of the pointers is non-null, but usually
> you'll get an arbitrary positive or negative result. On the other hand
> it wouldn't surprise me if the optimizer were allowed to make convenient
> assumptions about the truncation, and just re-use the sign of the
> original ptrdiff_t.
>
> Am I right in thinking this comparison is more or less arbitrary, as
> long as the result is consistent and only zero if the two pointers are
> both null? If so, would anyone mind if I made it compare just the
> nullness of the two pointers?
>
> The actual code may not end up looking like this, but I'm thinking along
> the lines of:
>
> «
> comp = (
> int(m_ffStates[i] == NULL) - int(compare.m_ffStates[i] == NULL)
> );
> »
>
> This way, a null pointer would be deterministically "less than" a
> non-null pointer.
>
> Jeroen
> _______________________________________________
> Moses-support mailing list
> Moses-support@mit.edu
> http://mailman.mit.edu/mailman/listinfo/moses-support [1]
Links:
------
[1] http://mailman.mit.edu/mailman/listinfo/moses-support
_______________________________________________
Moses-support mailing list
Moses-support@mit.edu
http://mailman.mit.edu/mailman/listinfo/moses-support
