[solved] comparing floating point with == or != is unsafe



  • Moin moin,

    bei einem kleinen Projekt bekomme ich diverse warnings: comparing floating point with == or != is unsafe.

    Es werden zwei floats verglichen, ich compiliere mit -pedantic -Wall -Wpedantic -Wextra und -std=c++17.

    Es handelt sich um eine theta-Funktion die ich mir aus nem alten Sedgewick geholt habe und etwas umgeschrieben habe, auto statt float z.B. (Dot ist ein QPointF aus Qt, also .x() und .y() gibt float zurück) :

    float theta(Dot &p1, Dot &p2)
    {
      auto dx = p2.x() - p1.x(); auto ax = std::abs(dx);
      auto dy = p2.y() - p1.y(); auto ay = std::abs(dy);
    
      auto t = ( ax + ay == 0 ) ? .0f :  dy / (ax+ay);  // Hier ist die Warnung
      if (dx < 0)
    [...]
    

    Wie mache ich es besser damit die warnings verschwinden? Der Code selbst macht das was er soll.

    cu

    Edit: QPointF ist ein qreal und hier bei mir double.



  • @dirkski sagte in comparing floating point with == or != is unsafe:

    Wie mache ich es besser damit die warnings verschwinden? Der Code selbst macht das was er soll.

    Muss er aber wohl nicht.

    Floating points sind binär immer als eine Art Annäherung des darzustellenden Wertes anzusehen. Wenn du das ganz richtig machen willst, musst du die Zahl in einen Bereich eingrenzen (x >= min && x <= max). Da gibt es was, das sich Epsilon nennt, kannst du mal nachsehen, findest im Internet bestimmt einen Haufen Anleitungen zu dem Thema.



  • @spiri Ja, epsilon, da war doch was. Die kleinste Zahl größer 1 glaub.

    Ich muss also testen ob der Wert irgendwo um 0 liegt, irgendwie sowas?
    also größer 0-epsilon und kleiner 0+epsilon. Ne, geht auch nicht kann dann ja nur 0 sein. Na ich werde erstmal was dazu lesen, solange bleibt der code. Die warnings laufen ja nicht weg... 🙂

    Danke vielmals

    PS: Wo ist das Syntax-Hilighting hin, heul...



  • @dirkski ah, jetzt ist es da. Nur in der Vorschau nicht...



  • Es gibt std::nextafter, um den nächstgrößeren Float-Wert heraufzubekommen. Angenommen nun, dass bei dir ax = 0 und ay = std::nextafter(0.f, 1) ist. Ist dann die Division sinnvoll - es wird dann durch 4.9e-324 geteilt. Ich könnte mir gut vorstellen, dass du auch da lieber noch 0 zurückgeben möchtest.
    Du könntest alternativ in der Umgebung der Nulldivision irgendwie eine andere Berechnung durchführen, d.h. irgendeine passende stabile Funktion wählen, die dein Problem approximiert. Oder eben einfach den Bereich, wo du 0 zurückgibst, ausweiten.



  • @wob sagte in comparing floating point with == or != is unsafe:

    Es gibt std::nextafter, um den nächstgrößeren Float-Wert heraufzubekommen. Angenommen nun, dass bei dir ax = 0 und ay = std::nextafter(0.f, 1) ist. Ist dann die Division sinnvoll - es wird dann durch 4.9e-324 geteilt. Ich könnte mir gut vorstellen, dass du auch da lieber noch 0 zurückgeben möchtest.

    In dem Fall ist aber auch dy = ±4.9e-324, also wäre das richtige Ergebnis dann ±1, was ziemlich weit entfernt von 0 ist?

    Letztendlich geht es doch nur darum die Division durch exakt 0 zu vermeiden, da auch Werte die extrem nah an 0 sind immernoch "plausible" Werte ergeben. Deswegen würde ich eher sagen die Warning ist hier nicht gerechtfertigt (wobei der Compiler das natürlich nicht wissen kann).

    Wenn man keine Probleme damit hat dass t sich "ähnlich" wie eine Signum-Funktion verhält und es nur darum geht die Warning zu vermeiden könnte man auch einfach den Code umstellen:

    auto t = ( ax + ay > 0 ) ? dy / (ax+ay) : .0f;
    

    Macht das selbe, nur ohne Warnung.



  • @happystudent Danke, das ist natürlich die Lösung. Vielen Dank.

    Noch was zum Hintergrund:

    Ich habe das letze mal vor 7 Monaten auf dem Code geguckt, daher stach mir das ins Auge. Das ist nur ein Hobby-Projektchen. Ich habe in einem alten Buch von Sedgewick gestöbert (Algorithmen in C++) und dachte mir ist zur Übung ganz gut das mal anzupassen. Ist sehr altes C++, eigentlich eher C.

    Im obigen post sind ein paar Fehler, QPointF hat qreal als Datentyp, der ist hier bei mir double. Auch ist mir gerade Aufgefallen das es hin und wieder falsche Berechnungen gibt, also tut nicht das was es soll 😃
    Werde da ein template raus machen und mit double probieren...

    Ich werde erst mal weiter schauen und bei weiteren fragen einen neuen Thread aufmachen, zumindest wenn es nicht genau um diesen Punkt geht...

    Angenehmen Sonntag noch an alle.



  • Euch ist aber schon klar dass 0 hier nicht in jedem Fall der einzige Wert ist der bei der Division zu einem Problem führt... ?



  • @hustbaer sagte in [solved] comparing floating point with == or != is unsafe:

    Euch ist aber schon klar dass 0 hier nicht in jedem Fall der einzige Wert ist der bei der Division zu einem Problem führt... ?

    Welche Werte sind denn noch problematisch? Meinst du Inf/NaN? Da im Original-Code keine checks dafür vorhanden waren, bin ich einfach mal davon ausgegangen dass diese Werte nicht auftreten können.



  • @hustbaer, @happystudent und alle:

    Tja, habe ich mir natürlich keine Gedanken drum gemacht 🙂

    Der Original-Code vom Sedgewick benutzt ein Ganzzahl-Koordinatensystem, ich hier double. Ich hänge mal ein ausführbares Beispiel dran mit dem Original und was ich mir gebastelt habe:

    // $Id$
    // g++-8 -pedantic -Wall -Wpedantic -Wextra  -std=c++17
    
    #include <cstdlib>
    #include <iostream>
    
    
    namespace sedgewick {
    
    // struct point, Sedgewick Kapitel 24, Seite 400
    struct point
    {
      int x, y;
      char c;
    };
    
    // theta() - R.Sedgewick - Algorithmen in C++ 5. Unveränderter Nachdruck 1999
    // Kapitel 24, Seite 405
    float theta(struct point p1, struct point p2)
    {
      int dx, dy, ax, ay;
      float t;
      dx = p2.x - p1.x; ax = abs(dx);
      dy = p2.y - p1.y; ay = abs(dy);
    
      t = (ax+ay == 0) ? 0 : (float) dy / (ax+ay);
      if (dx < 0)
        t = 2-t;
      else if (dy < 0)
        t = 4+t;
    
      return t*90.0;
    }
    
    } // namespace sedgewick
    
    
    //
    // Neue Version
    //
    
    namespace my {
    
    template <class T>
    struct Dot : public T
    {
      float found = -1;
      friend std::ostream& operator<<(std::ostream& os, const Dot& d)
      {
        return os << "[" <<d.x() << "; " << d.y() << "] " << d.found;
      }
    };
    
    template <typename T>
    auto theta(T &p1, T &p2) -> decltype( p1.x() )
    {
    //  static_assert (std::is_same_v<double, decltype ( p1.x() )>, "not double");
      auto dx = p2.x() - p1.x(); auto ax = std::abs(dx);
      auto dy = p2.y() - p1.y(); auto ay = std::abs(dy);
    
      auto t = ( ax + ay == 0 ) ? .0f :  dy / (ax+ay);
      if (dx < 0)
        t = 2-t;
      else if (dy < 0)
        t = 4+t;
    
      return t * 90.f;
    }
    
    
    } // namespace my
    
    
    // QPointF-Ersatz für Qt's QPointF. x,y sind qreal = double
    struct PointF
    {
      double _x=0, _y=0;
      double x() const { return _x; }
      double y() const { return _y; }
    };
    
    
    int main (int, char**)
    {
      sedgewick::point p1{0, 0, '1'}, p2{10, 10, '2'};
    
      std::cout << "Sedgewick:\nPunkt 1: x;y=[" << p1.x << "; " << p1.y
                << "]\nPunkt 2: x;y=[" << p2.x << "; " << p2.y
                << "], Winkel zu Punkt 1 = " << sedgewick::theta(p1, p2) << "°\n";
    
    
      // my
      using Point = my::Dot<PointF>;
    
      // Warnung: suggest braces around initialization of subobject
      Point pf1{{0., 0.}}, pf2 {{10., 10.}}; // Edit: = entfernt / Edit2: Klammern verdoppelt um obige Warnung abzustellen
    
      std::cout << "my:\nPunkt 1: x;y=" << pf1
                << "\nPunkt 2: x;y=" << pf2
                << ", Winkel zu Punkt 1 = " << my::theta(pf1, pf2) << "°\n";
    
      return EXIT_SUCCESS;
    }
    

    Ausgabe:

    Sedgewick:
    Punkt 1: x;y=[0; 0]
    Punkt 2: x;y=[10; 10], Winkel zu Punkt 1 = 45°
    my:
    Punkt 1: x;y=[0; 0] -1
    Punkt 2: x;y=[10; 10] -1, Winkel zu Punkt 1 = 45°
    

    Sollte ich in dem Fall noch auf NaN und Unendlichkeiten prüfen? Ich habe in einer darauf aufbauenden Funktion zum finden der Konvexen Hülle (wrap, einwickeln) gelegentlich einen Fehler gehabt. Allerdings habe ich die Berechnungen mit float durchgeführt obwohl das Kordinatensystem double war. Jetzt wo ich auf template umgestellt habe ist mir zumindest nichts mehr aufgefallen.

    cu

    PS: Soll ich das [solved] wieder aus dem Titel nehmen?

    Edit3: Codetags auf ```c++ geändert, dann wird auch namespace und template hervorgehoben 😎



  • @happystudent Versuch mal DBL_MAX durch 0.1 zu dividieren. Oder 1e200 durch 1e-200.


  • Mod

    @hustbaer sagte in [solved] comparing floating point with == or != is unsafe:

    @happystudent Versuch mal DBL_MAX durch 0.1 zu dividieren. Oder 1e200 durch 1e-200.

    Hier können wir als Mensch aber mitdenken und sehen, dass dy vom Betrag her stets kleinergleich (ax+ay) sein muss, das Ergebnis also schlimmstenfalls vom Betrag her 1 sein kann. Außer eben im Sonderfall dy=ax=ay=0.

    Zumindest solange niemand anfängt, der Funktion Punkt mit Koordinaten INF oder NAN vorzusetzen. In dem Fall wäre ich für das gute alte 'garbage in, garbage out'.



  • @seppj sagte in [solved] comparing floating point with == or != is unsafe:

    @hustbaer sagte in [solved] comparing floating point with == or != is unsafe:

    @happystudent Versuch mal DBL_MAX durch 0.1 zu dividieren. Oder 1e200 durch 1e-200.

    Hier können wir als Mensch aber mitdenken und sehen, ...

    Jaaaaa 🙂 Wenn ich mitgedacht hätte, dann hätte ich das vermutlich auch irgendwann gesehen.
    self- 🤦🏻♂


  • Mod

    @hustbaer sagte in [solved] comparing floating point with == or != is unsafe:

    @seppj sagte in [solved] comparing floating point with == or != is unsafe:

    @hustbaer sagte in [solved] comparing floating point with == or != is unsafe:

    @happystudent Versuch mal DBL_MAX durch 0.1 zu dividieren. Oder 1e200 durch 1e-200.

    Hier können wir als Mensch aber mitdenken und sehen, ...

    Jaaaaa 🙂 Wenn ich mitgedacht hätte, dann hätte ich das vermutlich auch irgendwann gesehen.
    self- 🤦🏻♂

    An sich war das aber ein guter Einwand. Ich bin mir ziemlich sicher, dass deinen Vorrednern entgangen ist, dass auch einige andere Divisionen allgemein problematisch sein können. Wenn auch nicht in diesem speziellen Fall.



  • @seppj Ich muss gestehen das ich mir den Sinn überhaupt nicht überlegt habe. Noch nicht mal das es sich um ein Test handelt damit es keine Division durch 0 gibt. Also das gilt für jetzt. Vor x Monaten schon. Mir sprang die Warnung ins Auge und war schon im Forum. Naja...

    cu


Log in to reply