Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uszu gets unreliable #993

Open
CaeruleusAqua opened this issue Jan 11, 2025 · 18 comments
Open

Uszu gets unreliable #993

CaeruleusAqua opened this issue Jan 11, 2025 · 18 comments

Comments

@CaeruleusAqua
Copy link
Contributor

CaeruleusAqua commented Jan 11, 2025

Hi folks,

I have noticed several times that uszu unfortunately often works unreliably. sometimes it just seems to miss things.
What is worse, however, is that there are some scripts that control the brightness depending on the position of the sun and uszu only makes nonsense.

I then tried to debug this and noticed that a Python thread was running amok.
Permanently 100% CPU.

I have now been able to narrow down the problem to the ephem library. The problem is that the calculation of the position of the sun and the moon converges badly under some constellations (_find_rise_or_set). Unfortunately, this also explains why uszu sometimes runs completely without problems. I have installed a counter as a test, and the function currently does not converge even over 100 million iterations, and blocks the complete uszu .

There are also some issue entries in ephem

brandon-rhodes/pyephem#266
brandon-rhodes/pyephem#232

Unfortunately, nothing has happened for years

@CaeruleusAqua
Copy link
Contributor Author

I have limited the number of iterations to 1000 as a test and everything seems to be working again. However, I have no idea how much this affects the calculation of the time points.

@bmxp
Copy link
Member

bmxp commented Jan 12, 2025

I thought that Skyfield could be a nice replacement for pyephem. But when I examined this some years ago it just was not ready and it needed external files to be downloaded from internet source so I decided to wait.
Maybe you want to try to exchange pyephem for another solution?

In the meantime: Would you mind share your changes? We could implement a fix at best

@onkelandy
Copy link
Member

Gow did you linit iterations? Would a parameter for the plugin be a solution?

@CaeruleusAqua
Copy link
Contributor Author

I have changed code directly in ephem, if then we would have to fork ephem, maybe we also get a pull request with a new parameter for the function through...

But I think the best thing would be to replace the lib...

ephem/init.py

    def _find_rise_or_set(self, body, start, use_center, direction, do_rising, max_iterations = 1000):
        if isinstance(body, EarthSatellite):
            raise TypeError(
                'the rising and settings methods do not'
                ' support earth satellites because of their speed;'
                ' please use the higher-resolution next_pass() method'
                )

        original_pressure = self.pressure
        original_date = self.date
        try:
            self.pressure = 0.0  # otherwise geometry doesn't work
            if start is not None:
                self.date = start
            prev_ha = None
            iterations = 0
            while True:
                iterations+=1
                if isnan(self.date):
                    raise ValueError('cannot find a next rising or setting'
                                     ' if the date is NaN')
                body.compute(self)
                horizon = self.horizon
                if not use_center:
                    horizon -= body.radius
                if original_pressure:
                    horizon = unrefract(original_pressure, self.temp, horizon)
                abs_target_ha = self._target_hour_angle(body, horizon)
                if do_rising:
                    target_ha = - abs_target_ha  # rises in east (az 0-180)
                else:
                    target_ha = abs_target_ha    # sets in west (az 180-360)
                ha = body.ha
                difference = target_ha - ha
                if prev_ha is None:
                    difference %= tau  # force angle to be positive
                    if direction < 0:
                        difference -= tau
                    bump = difference / tau
                    if abs(bump) < default_newton_precision:
                        # Already at target event: move forward to next one.
                        bump += direction
                else:
                    difference = _plusminus_pi(difference)
                    bump = difference / tau
                if abs(bump) < default_newton_precision:
                    break
                if iterations > max_iterations:
                    break;
                self.date += bump
                prev_ha = ha

            if abs_target_ha == _slightly_more_than_pi:
                raise AlwaysUpError('%r is above the horizon at %s'
                                    % (body.name, self.date))

            if abs_target_ha == _slightly_less_than_zero:
                raise NeverUpError('%r is below the horizon at %s'
                                   % (body.name, self.date))

            return self.date
        finally:
            if self.pressure != original_pressure:
                self.pressure = original_pressure
                body.compute(self)
            self.date = original_date

@CaeruleusAqua
Copy link
Contributor Author

Hey guys,

I have some time right now. So I've started replacing ephem with skyfield.
I have a few questions about the functions modnight and noon.

At least in ephem they seem to return the times for the current day. However, the corresponding skyfield methods return the next time after the given time.

Which of these is the intended behavior?

The code is currently located at:

https://github.com/CaeruleusAqua/smarthome_skyfield

@bmxp
Copy link
Member

bmxp commented Feb 11, 2025

Ich glaube es ist einfacher in Deutsch zu schreiben als das ganze in DEnglish zu machen :-)

Skyfield bietet die Möglichkeit eine einem Rutsch eine bestimmte Zeitspanne nach risings und settings zu durchsuchen, und ebenfalls nach noon und midnight.

Mein Ansatz war es ein ganzes Jahr beim Start zu berechnen für

  • maximale Tiefe
  • maximale Höhe
  • übliche sonnenaufgangs- und sonnenuntergangspunkte (nautisch, astronomisch, etc.)
  • Bei Anforderung von weiteren Zwischenpunkten diese zu cachen
    und zwischenzuspeichern in var/cache/orbs

Es gibt zwei Sachen die mich damals davon abgehalten haben skyfield einzusetzen:

  1. Der Algorhythmus in Skyfield für die Berechnung hatte noch Fehler, er ist irgendwann ins Nirwana weitergegangen. Ich hatte da mit dem Ersteller Kontakt und das ist wohl behoben mittlerweile.
  2. Es wird eine Datei aus dem Internet benötigt. Es ist damals unklar gewesen welche Quelle dafür genutzt werden kann. Ich weiß nicht ob es a) dafür nun eine Festlegung gibt und ob b) ein Fallback existiert damit man eine solche Datei mitliefern kann mit der Installation
  3. Man wird nie den exakten Zeitpunkt ermitteln können sondern nur bis auf ein sagen wir mal Epsilon. Die Frage ist in dem Fall in wie weit das Epsilon in weiteren Orb Routinen Nebeneffekte durch Rundungen hat.

Ein Vorteil ist das schöne API von Skyfield. Es ist sauber vorbereitet und man kann recht sicher sein das es nicht so von deprecations wimmelt wie andere Module.

@CaeruleusAqua
Copy link
Contributor Author

Welchen Vorteil versprichst du dir von diesem Ansatz?

Was ich festgestellt habe, ist, dass man immer innerhalb eines Zeitfensters suchen muss, das entsprechend groß gewählt werden muss. (evtl. problematisch in Polnähe) In ephem konnte direkt nach dem nächsten Event gesucht werden..

  1. Soweit ich gelesen habe, wurde die API von Skyfield überarbeitet, in der Dokumentation steht auch, dass die alte noch funktioniert, aber Fehler hat. (soviel zu den Deprecations)
  2. Die Datei wird jetzt vollautomatisch heruntergeladen, im Prinzip könnte man die Daten aber auch manuell ausliefern.
  3. Das Problem dürfte bei jeder Lib auftreten, nicht nur bei Skyfield.

@bmxp
Copy link
Member

bmxp commented Feb 11, 2025

Skyfield gibt auf Anfrage die risings und settings für ein ganzes Jahr zurück. Und da die Werte immer wieder angefordert werden sollten die Ergebnisse halt gecached werden. Es ist nicht notwendig sunrise 10 mal am Tag zu ermitteln nur weil es irgendwo in der Logik steht.
Das API sollte auch daraufhin erweitert werden ob man den nächsten Sonnenaufgang wissen will der überhaupt in der Zukunft liegt oder ob man den heutigen Sonnenaufgang wissen will. Ersteres ist derzeit wohl das Verhalten von SHNG, letzteres wäre wünschenswert zu haben.
Wenn Du also eine Liste mit Sonnenauf- und Untergangszeitpunkten hast kannst Du einfach nachsehen.
Ach ja, ich hatte damals gesehen das einige Leute Probleme mit Ihrem Raspi hatten weil sie SciPy/NumPy nicht installiert bekommen haben. Das war zu der Zeit für UZSU genutzt worden um Approximation der Kurven zu haben.
NumPy/Scipy ist halt ein Monster und das wollte ich nicht ohne triftigen Grund heraufbeschwören.
Wie das aktuell ist, weiß ich nicht.

@CaeruleusAqua
Copy link
Contributor Author

CaeruleusAqua commented Feb 11, 2025

Skyfield gibt auf Anfrage die risings und settings für ein ganzes Jahr zurück. Und da die Werte immer wieder angefordert werden sollten die Ergebnisse halt gecached werden. Es ist nicht notwendig sunrise 10 mal am Tag zu ermitteln nur weil es irgendwo in der Logik steht.

Kann ich machen.

Das API sollte auch daraufhin erweitert werden ob man den nächsten Sonnenaufgang wissen will der überhaupt in der Zukunft liegt oder ob man den heutigen Sonnenaufgang wissen will. Ersteres ist derzeit wohl das Verhalten von SHNG, letzteres wäre wünschenswert zu haben.
Wenn Du also eine Liste mit Sonnenauf- und Untergangszeitpunkten hast kannst Du einfach nachsehen.

Das Problem ist, dass es Regionen auf der Erde gibt, in denen es nicht jeden Tag einen Sonnenaufgang/-untergang gibt, sodass die Abfrage nach dem nächsten Sonnenaufgang zum Zeitpunkt T wohl die geeignetere Variante ist.

Ach ja, ich hatte damals gesehen das einige Leute Probleme mit Ihrem Raspi hatten weil sie SciPy/NumPy nicht installiert bekommen haben. Das war zu der Zeit für UZSU genutzt worden um Approximation der Kurven zu haben.
NumPy/Scipy ist halt ein Monster und das wollte ich nicht ohne triftigen Grund heraufbeschwören.
Wie das aktuell ist, weiß ich nicht.

Ich habe NumPy vor ewigen Zeiten auf dem RPI1 benutzt, da gab es tatsächlich einige Herausforderungen.
Aktuell kann ich mir das aber nicht mehr vorstellen (habe aber leider auch keinen RPI zum Testen hier).

Nachdem ich jetzt Skyfield gegen Ephem getestet habe, komme ich immer mehr zu dem Schluss, dass es eine gute Idee ist Ephem zu ersetzen.

Ephem, macht sehr merkwürdige Dinge, wenn ich den nächsten Sonnenaufgang für 5 Uhr anfrage und der Sonnenaufgang war schon um 4:49, bekomme ich trotzdem 4:49. Erst ab 8 Uhr bekomme ich den für den nächsten Tag, das ergibt keinen Sinn.
Die berechneten Werte für Azimut und Altitude stimmen auch nicht, zumindest nicht, wenn ich Skyfield und https://www.sonnenverlauf.de als Referenz nehme...

Leider ist Skyfield deutlich langsamer, das cachen macht daher auch sin.

Ich habe heute noch Zeit ein oder zwei Dinge umzusetzen, danach geht mein Zeitbudget für private Projekte leider wieder gegen 0.
Wenn ihr also größere API Änderungen wollt, müsstet ihr das wahrscheinlich selber übernehmen.

@CaeruleusAqua
Copy link
Contributor Author

So wie es aussieht, liegt das Problem mit Azimut und Altitude nicht in Ephem, sondern durch die inkorrekte Umwandlung der Zeitzonen (dt.replace(tzinfo=tzutc())) mit astimezone(datetime.UTC) ist alles korrekt.

@CaeruleusAqua
Copy link
Contributor Author

So wie es aussieht, funktioniert die _avoid_neverup funktion nicht aussreichend.

Wenn ich doff in rise auf 90 setze, bekomme ich genau den Fehler, für den ich das Issue geöffnet habe.
Wenn ich den Wert von _avoid_neverup mit 0.99 multipliziere, scheint es zu funktionieren.
Scheint ein nummerische Problem zu sein.

@bmxp
Copy link
Member

bmxp commented Feb 11, 2025

Deine Beobachtung mit _avoid_never_up kann ich nachvollziehen. Wobei man die Funktion dann eigentlich nicht mehr benötigen würde, sie ist ja nur reingekommen um es abzufangen das die Sonne z.B. in Hammerfest im Juli niemals untergeht.

Wenn auf Skyfield umgestellt ist, wird man die Funktion nicht mehr benötigen.

Der Winkel der minimalen/maximalen Sonnenwinkel ist ja auch nicht unbedingt zu Mitternacht/High Noon erreicht sonder je nach Wohnort etwas anders als die Berechnung.

Schau einfach wie weit Du für Dich kommst und teste ob es für Dich klappt. Ich schaue mir Deinen Code gerne an wenn ich wieder etwas Luft habe, dann können wir Ephem im Code durch Skyfield ersetzen. Ich würde das aber vor dem nächsten Release nicht unbedingt mehr angehen wollen.

@msinn
Copy link
Member

msinn commented Feb 11, 2025

Moin,
Das betrifft aber nicht nur Hammerfest (sondern auch die Hammerburg 😉)

Ich hatte in/für Hamburg Probleme, wenn ich mich nicht nur für die bürgerliche Dämmerung interessierte, sondern für die astronomische Dämmerung (wenn also die Sonne 18° unter dem Horizont steht). Dann bekam ich früher im Sommer den Fehler, dass die Sonne nie unter dem Horizont stand.

@CaeruleusAqua
Copy link
Contributor Author

Dann müsste man ggf. einige weitere neue Events einbauen.

Denn über die Liste mit den Sonnenuntergängen bekommt man halt nur die konkrete Zeitpunkte.

In USZU kann man jedoch beliebige Offsets (inkl. Grad und Minuten) für den Sonnenstand einbauen.
Das zu entfernen, dürfte einige Installationen kaputt machen.

Ich z.b. nutze die Funktion für mein HCL-Lighting.

@CaeruleusAqua
Copy link
Contributor Author

@msinn genau sowas meine ich...

@bmxp
Copy link
Member

bmxp commented Feb 11, 2025

Ja, das ist korrekt. Nicht nur die Sonnenauf und Untergänge sondern die Anfragen die reinkommen sollten gecached werden. Aber das kann man ggf. auch später machen.

@CaeruleusAqua
Copy link
Contributor Author

Weitere Frage:
Sollte der "doff" parameter in z.B: der Rise Funktion der Offset zu 0° Azimut oder zum Sonnenauffgang sein?

Aktuell ist es nämlich letzteres, ersteres wäre aber intuitiver (off =-12 wäre dann z.B. Nautische Dämmerung)

@CaeruleusAqua
Copy link
Contributor Author

Ist jetzt soweit umgesetzt,
wenn ihr noch was braucht lasst es mich wissen..

BTW:
Wenn in der _avoid_neverup die numerische Toleranz erhöht wird, ist die Endlosschleife zumindest in meinen Tests behoben:
also:

# Limit degree offset to the highest or lowest possible for the given date
    doff = max(doff, max_altitude + 0.00001) if doff < 0 else min(doff,
                                                                  max_altitude - 0.00001) if doff > 0 else doff

zu

    # Limit degree offset to the highest or lowest possible for the given date
    doff = max(doff, max_altitude + 0.01) if doff < 0 else min(doff,
                                                                  max_altitude - 0.01) if doff > 0 else doff

ggf. schafft es der Fix ja in nächste release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants