Mittwoch, 6. August 2014

Refaktorisierung von sweph()

Aufs Geratewohl herausgegriffen aus den verbliebenen knapp 5'000 Zeilen sei die Funktion sweph(). Ich stiess auf sie, weil sie von oben die erste Funktion ist, die die goto-Anweisung verwendet. Sicher ist goto considered harmful (Dijkstra) und sollte daher vermieden werden. Wichtiger ist aber die Verletzung des Single Responsibility Principle (SRP), dass eine Funktion nur für eine einzelne Aufgabe zuständig sein sollte. Das gilt ebenso für alle anderen Funktionen dieses Codes, aber irgendwo muss man einmal anfangen.

Die Funktion sweph() berechnet aus den Files der Swiss Ephemeris die baryzentrische Position des gewünschten Objekts. Gemäss ihrem einleitenden Kommentar hat sie folgende Aufgaben:

/*
 * this function looks for an ephemeris file,
 * opens it, if not yet open,
 * reads constants, if not yet read,
 * computes a planet, if not yet computed
 * attention: asteroids are heliocentric
 *            other planets barycentric
*/
Bei genauerer Inspektion des Codes führt sie die folgenden Aufgaben aus:
  • Sie puffert den letzten Aufruf pro Planet. Die Struktur swed enthält für jeden Planeten einen "Slot" mit den Ergebnissen des letzten Aufrufs.
  • Sie invalidiert den Zeiger auf das Ephemeris File, falls dieses von Datum oder Planetennummer nicht mehr gültig ist. In diesem Fall wird das File auch mit fclose() geschlossen und allokiertes Memory freigegeben.
  • Ist das File ungültig, werden drei mögliche Filenamen für den korrekten Kandidaten ermittelt:
    1. Standardnamen wie sepl_18.se1 oder ast0/se000302.se1,
    2. sogenannte Kurzversionen mit einem "s" vor der File Extension wie in ast0/se00005s.se1,
    3. und schliesslich, falls der Pfad ein Verzeichnis enthielt wie in ast0/se000302.se1, wird ein letzter Versuch für den Filenamen ohne Verzeichnis gemacht, im Beispiel also se000302.se1.
    Die Kandidaten 2 und 3 werden dabei nur für Asteroiden probiert. Es wird versucht, Files mit diesem Namen zu öffnen. Das erste File, für das dies gelingt, wird das neue "aktuelle Ephemeris File".
  • Es werden die für das ganze File gültigen Konstanten in die swed-Struktur eingelesen.
  • Es wird am Anfang und Ende des Gültigkeitsbereichs der Swiss Ephemeris eine Randbedingung fürs Datum geprüft.
  • Nun werden die Koeffizienten des für das Julianische Datum gültigen Interpolationspolynoms eingelesen.
  • Das Polynom wird evaluiert.
  • Für den Sonderfall der baryzentrischen Sonne wird die gesamte Funktion sweph() noch einmal für das Erde-Mond-Baryzentrum aufgerufen und das Ergebnis vektoriell zur Position der heliozentrischen Sonne addiert.
  • Die Asteroidenpositionen sind heliozentrisch und werden in die baryzentrische Positionen konvertiert.
  • Zum Kennzeichen, dass die berechneten Positionen gemerkt werden, wird die Zahl pdp->teval auf das Julianische Datum der Anfrage gesetzt.
Für eine Funktion mit 185 Zeilen sind das ziemlich viele Aufgaben. Auch gehören die Aufgaben zu verschiedenen Ebenen:
  • Ebene File: Datei öffnen und schliessen, Filenamen
  • Abbildung von Filedaten in Datenstruktur: Dateiweite Konstanten und Koeffizienten des zum aktuellen Objekt / Datum gültigen Interpolationspolynoms.
  • Berechnung der baryzentrischen Position: Auswertung des Interpolationspolynoms, Umrechnung von Asteroidenpositionen von helio- in baryzentrisch.
  • Pufferung der Abfrage
Nach Refaktorisierung dokumentiert der Code in der obersten Ebene die einzelnen Aufgaben, aus denen die Funktion besteht:
static int sweph(double tjd, int ipli, int ifno, int iflag, 
                 double *xsunb, AS_BOOL do_save, double *xpret, 
                 char *serr, struct swe_data *swed) {

  int ipl = is_main_object(ipli) ? ipli : SEI_ANYBODY;
  struct plan_data *pedp = &swed->pldat[SEI_EARTH];
  struct plan_data *psdp = &swed->pldat[SEI_SUNBARY];
  struct plan_data *pdp = &swed->pldat[ipl];

  /* In do_save mode, use swe_data memory slot for results */
  double* xp = do_save ? pdp->x : xpret;

  /* if planet has already been computed for this date, return */
  if (sweph_isPositionInBuffer(tjd, ipl, iflag, pdp, xpret))
    return (OK);

  /* Get ephemeris file handle */
  struct file_data* fdp = 
    sweph_getEphemerisFile(tjd, ipli, ifno, pdp, swed, serr);
  if (fdp == NULL )
    return NOT_AVAILABLE;

  if (sweph_isDateOutOfBound(tjd, fdp, serr, swed))
    return NOT_AVAILABLE;

  /* Read coefficient data */
  sweph_getCoefficientSegment(tjd, ipl, ifno, pdp, serr, swed);

  /* Also compute speed, if
   * 1. true position is being computed before applying light-time etc.
   *    this is the position saved in pdp->x.
   *    in this case, speed is needed for light-time correction.
   * 2. the speed flag has been specified.
   */
  bool need_speed = do_save || is_set(iflag, SEFLG_SPEED);

  /* Compute position */
  sweph_evaluateChebyshevPolynomial(tjd, pdp, need_speed, xp);

  /* Special case barycentric Sun */
  if (ipl == SEI_SUNBARY && (pdp->iflg & SEI_FLG_EMBHEL))
    sweph_barycentricSunFromEarth(tjd, ifno, iflag, need_speed, 
                                  pedp, xp, serr, swed);

  /* asteroids are heliocentric.
   * if JPL or SWISSEPH, convert to barycentric */
  if ((ipl >= SEI_ANYBODY) && 
      is_set(iflag, SEFLG_JPLEPH | SEFLG_SWIEPH))
    vec_add(xp, xsunb, need_speed ? 6 : 3);

  /* Mark result in save area as valid buffer */
  if (do_save) {
    sweph_validateBuffer(tjd, ifno, pdp, psdp);
    vec_copy(xpret, xp, 6);
  }

  return (OK);

}
Zwei Ebenen tiefer finden wir die zuvor mit goto again gelöste Aufgabe, das Ephemeridenfile anhand der drei Kandidaten für den Namen zu ermitteln. Die Code-Ersparnis, die das goto motiviert hatte, erwies sich als nicht besonders hoch: Man sparte sich nur die eine Codezeile mit dem Aufruf von swid_fopen(). Dadurch, dass es nun eine eigene kleine Funktkion, zwei Ebenen unterhalb von sweph() gibt, können wir return wie ein break benutzen, um auszusteigen, sobald swid_fopen() einen gültigen Filepointer lieferte:
static FILE* sweph_openEphemerisFile(int ipli, int ifno, char *fname, 
                                     char *serr, struct swe_data *swed) {
 
  FILE *fptr = swid_fopen(ifno, fname, swed->ephepath, serr, swed);
  if (fptr)
    return fptr;
 
  if (!is_main_object(ipli)) {
 
    char *sp;

    /*
     * if it is a numbered asteroid file:
     * try also for short files (..s.se1)
     */
    char s[AS_MAXCH];
    strcpy(s, fname);
    if ((sp = strrchr(s, '.'))) { 
      strcpy(sp, "s." SE_FILE_SUFFIX); /* insert an 's' */
      if ((fptr = swid_fopen(ifno, s, swed->ephepath, serr, swed)))
        return fptr;
      }
 
    /*
     * if we still have 'ast0' etc. in front of the filename,
     * we try in the main ephemeris directory instead of the
     * asteroid subdirectory.
     */
    if ((sp = strrchr(fname, *DIR_GLUE))) {
      if ((fptr = swid_fopen(ifno, sp+1, swed->ephepath, serr, swed)))
        return fptr;
    }
  
  }
  return NULL ;
}
Um festzustellen, wie Code für die Pufferung durchlaufen wird, schrieb ich einen neuen Test test_identical_call() für die Testsuite, der die Funktion se_calc zweimal nacheinander mit identischen Parametern aufruft. Überraschenderweise lieferte die Funktion sweph_isPositionInBuffer immer false!

Grund dafür war, dass exakt der gleiche Code, um festzustellen, ob das Ergebnis sich bereits im Puffer befindet, auch auf höherer Ebene bereits programmiert war - in der darüberliegenden Funktion sweplan(). Ich konnte diesen duplikativen Code nun durch Aufruf der Bedingungsfunktion ersetzen. Insgesamt gab es 6 identische Codestrecken in sweph.c, die jeweils diese Abfrage auf den Puffer machten, die ich nun alle durch den Aufruf der Bedingungsfunktion ersetzen konnte. Solche kleinen Bedingungsfunktionen werden übrigens vom Compiler meist automatisch ge-inlined. Die Bedenken, dass man durch den Funktionsaufruf eine zusätzliche Stackebene aufbauen und somit kostbare Rechenzeit verlieren würde, sind daher unbegründet.

Samstag, 22. Februar 2014

Die 90% Marke ist erreicht

Mit einer Abdeckung von 90.17% habe ich nun den Löwenanteil meiner Version test_calc_reduce.c (aus einer Kopie von sweph.c hervorgegangen) abgedeckt. Dafür werden insgesamt 1194 Testaufrufe der Swiss Ephemeris mit den verschiedensten Parametern ausgeführt, wie das folgende Kommando ergibt:
test_calc_reduce | perl -ne 'BEGIN { $count = 0;} $count += $1 if /(\d+) OK/; END { print qq(Total number of tests: $count\n); }'
Selbstverständlich sind alle API-Funktionsaufrufe enthalten - für die Planetenberechnungen auch mit einer grossen Bandbreite verschiedener Zeiten, verschiedener Flags, verschiedener Planeten. Um auch den Fall abzufangen, dass bei beschädigten Ephemeridenfiles korrekt ein Fehler gemeldet wird, habe ich sogar die Ephemeridendatei sepl_18.se1 in zwei automatischen Tests mit präparierten Versionen ausgetauscht.

Was nun übrig bleibt an nicht abgedecktem Code, betrifft

  • Ausnutzung von Pufferungen: Wenn ein in zeitlicher Nähe liegender Planetenstand zuvor schon einmal berechnet wurde, können Teile der Daten von der letzten Rechnung wiederverwendet werden. Die Zweige, die auf die Pufferung zugreifen, werden in meinen Tests nicht durchlaufen, da ich zwischen den Aufrufen stets die globale Struktur swed initialisiere. Ich könnte zwar in den Testfällen ein Flag clear_buffer vorsehen (das standardmässig gesetzt ist, aber abgeschaltet werden kann), um auch diese Codezweige zu testen. Aber ich will das gar nicht. Pufferungen werde ich nämlich ignorieren, entfernen und bei Bedarf hinterher einfügen, wenn es eine Performanceanalyse als nötig zeigt.
  • Extrem seltene Fehlersituationen: Beispielsweise ein ungültiger Koeffizient mitten im File sepl_18.se1.
  • Grundsätzlich nie durchlaufene Codestellen: Z.B. eine Codestrecke für den Fall baryzentrischer Moshierephemeriden, die aber gar nicht vorgesehen sind und daher weiter oben schon abgefangen werden. Oder eine Fehlermeldung, falls in einer Routine zur Sonnenberechnung nicht wirklich die Sonne
Bei diesem Stand der Testüberdeckung vom 22.2.2014 werde ich es nun im wesentlichen belassen (die Auswertungsfiles wurden mit dem Programm lcov aus den gcov-Resultaten erzeugt). Das Risiko, einige Randwerte beim Refaktorisieren falsch zu behandeln, ist zwar vorhanden, aber es lähmt mich nicht!

Mittwoch, 22. Januar 2014

Initialisierung ausschalten

Das Ausschalten der Initialisierung
    if (0) {
    swe_close( );
    const struct swe_data swed0 = {
      .ephe_path_is_set = false,
      .jpl_file_is_open = false,
      .fixfp = NULL,
      .ephepath =  SE_EPHE_PATH,
      .jplfnam = SE_FNAME_DFT,    /* JPL file name, default */
      .geopos_is_set = false,   /* geopos is set, for topocentric */
      .ayana_is_set = false     /* ayanamsa is set */
      };
    memcpy(&swed,&swed0,sizeof(struct swe_data));
    }
treibt die Abdeckung um ein halbes Prozent in die Höhe. Vermutlich ist es anderer Code, der in diesen Fällen durchlaufen wird. Die Tests bleiben aber alle auf OK, was beruhigend ist: Es gibt anscheinend keine weiteren verschleppten Abhängigkeiten mehr, die die Ergebnisse der Tests beeinflussen könnten.

Sonntag, 12. Januar 2014

82 Prozent

Ich habe nun den Testgenerator erweitert, so dass auch Testdaten für siderische Positionen generiert werden können. Ebenso kann eine topozentrische Position in der Fixture angegeben werden, so dass topozentrische Planetenberechnungen in der Fixture zugänglich sind. Für die Fixsterne und ihre Grössen, für die Ayanamsa-Namen, Planetennamen und die Zeitgleichung erlaube ich mir, die Tests und Erwartungen in test_test_calc_reduce.c hart zu codieren. Sie kommen im Testprotokoll immer am Schluss, nach den Tests aus der Fixture

Und so sieht das Protokoll nun aus.

ruediger@herschel:~/workspace/swepar/src$ test_calc_reduce
geocentric positions: 250 OK   0 NOT OK
helio/barycentric positions: 285 OK   0 NOT OK
barycentric Sun: 5 OK   0 NOT OK
astrometric positions J2000: 90 OK   0 NOT OK
topocentric positions: 15 OK   0 NOT OK
sidereal positions with Lahiri Ayanamsha: 25 OK   0 NOT OK
sidereal positions / Suryasiddhanta, strictly computed: 25 OK   0 NOT OK
individual ayanamsa: 25 OK   0 NOT OK
sidereal rectangular geocentric: 40 OK   0 NOT OK
Asteroid Urania: 5 OK   0 NOT OK
Hamburger Poseidon: 5 OK   0 NOT OK
different coordinate systems: 4 OK   0 NOT OK
Obliquity of ecliptic and nutation: 1 OK   0 NOT OK
Moshier, geocentric positions: 140 OK   0 NOT OK
Moshier, heliocentric positions: 100 OK   0 NOT OK
Provoking a "file not found" error: 1 OK   0 NOT OK
swe_calc_ut: 1 OK   0 NOT OK
New mode: Barycentric, directly from file: 1 OK   0 NOT OK
Fixed Stars: 7 OK   0 NOT OK
Get Ayanamsa: 6 OK   0 NOT OK
Get Planet Names: 3 OK   0 NOT OK
Equation of Time for J2000: 1 OK   0 NOT OK
Die damit erreichte Testabdeckung liegt bei 82 Prozent. In dem arg zerklüfteten gcov-File gibt es jetzt keine Hauptfunktion mehr, die nicht abgedeckt wäre. Die vielen kleinen roten Einsprengsel entstehen:
  • durch zahllose nicht durchlaufene Ausnahmesituationen (z.B. "Ephemeride nach Moshier darf nicht benutzt werden für baryzentrische Koordinaten"). Einiges davon hat mit nicht mehr unterstützten Optionen zu tun.
  • durch nicht ausgenutzte Puffer: Da ich zwischen den Tests normalerweise nun swe_close() aufrufe, wird der Code zum Wiederverwenden bereits berechneter Positionen nicht durchlaufen. Dies könnte ich noch mit einer weiteren kleinen Testserie erreichen.

Freitag, 10. Januar 2014

Der Nageltest für die Local History

Gleich nach meinem gestrigen Lobpreis auf die Local History konnte ich nun die Probe aufs Exempel machen. In der Konsole hatte ich versehentlich ein rm test_calc* ausgeführt, was mir ohne Sicherungen einige Backup-Arbeit beschert hätte. Nun konnte ich auf die Local History zurückgreifen: Sie erschien wieder, nachdem ich die Files test_calc_reduce.c und test_calc_reduce.h wieder leer angelegt hatte.

Nach einigen kleineren Macken konnte ich nun fünf Fixsterntests und vier Ayanamsa-Tests hinzufügen, was die Abdeckung zwar nicht weltbewegend hochgetrieben hat (sie liegt stieg von 68% auf 71% an), aber mit swe_fixstar() und swe_deltat() sind nun zwei prominente Funktionen mit im Rennen.

Die folgende Übersicht zeigt,

  • dass im Bereich der Berechnung von Fixsternpositionen und -grössen noch besonders viel Luft ist.
  • lunar_osc_elem könnte mit ein paar Lilith-Berechnungen besser ausgebeutet werden.
  • sweph, die zentrale Funktion für die Planetenberechnungen, ist erst zu drei Vierteln abgedeckt, obwohl sie mit den bestehenden Tests bereits über 6000 Mal aufgerufen wird.
  • Der Code, der die Anfrage aus dem Puffer bedient, wird nicht durchlaufen, da ich bislang keine zwei identische Positionsanfragen nacheinander stelle.
  • Ausserdem werden keine Asteroiden abgefragt, für die eines der ast-Subdirectories abgefragt werden müsste.
  • Die Abdeckung lässt sich deutlich steigern, wenn noch ein paar topozentrische Berechnungen dazukommen (vor allem durch swid_get_observer).
  • Die Zweige, die Planetenberechnungen mit Ayanamsa enthalten, werden noch nicht durchlaufen.

Donnerstag, 9. Januar 2014

Local History

Hätte ich die Local History von Eclipse schon 2012 gekannt, dann hätte ich nicht folgende Frage im Forum der Clean Code Developer abgesetzt:

Delta-Backup für alle Änderungen an Textdateien

Genau die Umkehrung des üblichen Master/Slave-Verhältnisses leistet die Local History, die für mich ohne weiteres Zutun von Eclipse verwaltet wird. Alles, was ich brauche, kann ich damit machen: Versionen vergleichen (und zwar jeden beliebigen Stand, ohne dass ich Commits einchecken und mit einem Text versehen muss), und bei Bedarf auch alte Versionen wieder zurückholen.

Wenn ich will, kann ich sogar einzelne Deltas aus früheren Versionen wieder holen. Alles sehr komfortabel und nützlich.

Mittwoch, 8. Januar 2014

Über die Benutzung vorgesehener Funktionen. Coverage jetzt 68%

Stand gestern: In der bash ausgeführt, lieferte das Programm test_calc_reduce unsinnige Testergebnisse, während es in Eclipse mit dem Pfad Run as->Local C/C++ Application korrekt ausgeführt wurde. Wie ist dieser Unterschied zu erklären?

Zwischen den Tests hatte ich ja die Struktur swed explizit initialisiert.

    const struct swe_data swed0 = {
      .ephe_path_is_set = false,
      .jpl_file_is_open = false,
      .fixfp = NULL,
      .ephepath =  SE_EPHE_PATH,
      .jplfnam = SE_FNAME_DFT,    /* JPL file name, default */
      .geopos_is_set = false,   /* geopos is set, for topocentric */
      .ayana_is_set = false     /* ayanamsa is set */
      };
    memcpy(&swed,&swed0,sizeof(struct swe_data));
Indem ich nun dieser Initialisierung noch einen Aufruf von swe_close() voranstellte, funktionierte das Programm mit beiden Aufrufmethoden wieder gleich. Es lohnt sich vermutlich nicht, der genauen Ursache nachzugehen, warum es zu diesem Fehlverhalten kam. Denn selbst wenn ich es vergessen haben sollte, ein globales Datum zu löschen: Warum verhielt sich das Programm dann beim Aufruf aus Eclipse anders als beim Aufruf aus der Konsole?

Anderes Thema: Ich habe einmal den Coverage Analyzer gcov angeworfen und festgestellt, dass ich mit der Testabdeckung für sweph.c (also mein test_calc_reduce.c) schon ganz gut liege:

Die vertikale Seitenleiste in der Abdeckungs-Quelltextanzeige zeigt, dass es vor allem im unteren Teil des Files noch viel Rot gibt: da sind einige Funktionen, die noch gar nicht aufgerufen werden. Das topozentrische Koordinatensystem habe ich noch mit keinem Testfall angefordert. Die vielen kleinen Splitter zwischendurch sind Ausnahmefälle wie: Die Fehlermeldung bei der Chironberechnung für ein Datum kleiner als CHIRON_START. Oder ein File, das behauptet, Big Endian zu sein, aber es in Wahrheit nicht ist, weil schon seine eigene Grösse in falscher Codierung abgespeichert ist. Hier werde ich irgendwo einen Stopp machen. Ich könnte natürlich in einem Test ein präpariertes File in den Ephemeridenpfad schieben, das dort das entsprechende korrekte File ersetzt. So könnte ich auch viele dieser Splitter noch "erschlagen".

Eine 100%-Abdeckung bekomme ich voraussichtlich nicht hin. Aber ab 95% könnte ich mit gutem Gefühl damit beginnen, auch aggressivere Umbaumassnahmen in meinem "Branch" test_calc_reduce.c vorzunehmen. Und mir dabei ziemlich sicher zu sein, dass die Kernfunktionen der Swiss Ephemeris erhalten bleiben.