Bitte Kritik zu meinem Code

Registriere dich jetzt, um exklusive Vorteile zu genießen! Als registriertes Mitglied kannst du Inhalte herunterladen und profitierst von einem werbefreien Forum.
Mach mit und werde Teil unserer Community!
  • Hallo,


    ich möchte euch bitten, mir als Programmierneuling Rückmeldung zu meinem Code zu geben. Der Hintergrund ist folgender:
    Ich beschäftige mich in meinem Studium mit Zahlen der Form n = h * 2^k + 1, wobei h ungerade und kleiner 2^k ist. Nun kann man prüfen, ob die Zahl n eine Primzahl ist:

    1) Gehe die Primzahlen sukzessive durch und bilde für jede Primzahl D das Jacobisymbol (n, D).

    2) Ist das Jacobisymbol 1, gehe eine Primzahl weiter

    3) Ist es 0, ist n keine Primzahl

    4) Ist es -1, berechne die Kongruenz D^((n-1)/2) modulo n. Kommt dort -1 raus, ist n prim. Sonst nicht


    Ich würde meinen Code mal päckchenweise hier posten in der Reihenfolge wie ich auch programmiert habe. Hoffe das ist so ok.

    Hier aber auch als Spoiler der gesamte Code:


    Ich habe mit einem anderen Skript die Primzahlen bis 10^7 gesiebt und in der Datei abgelegt. Dazu habe ich mir die beiden Funktionen listsave und listload geschrieben. Falls jemand den Code testen will, hänge ich die Datei gerne an.



    Dann berechne ich das Jacobisymbol. Ist jetzt vielleicht zu mathematisch, das hier zu erklären. Aber die Werte die dort herauskommen habe ich auch mit denen von sympy.ntheory.residue_ntheory.jacobi_symbol(m, n) verglichen. Ob letztere schneller ist als meine Version ist mir nicht so wichtig, da ich das hier erstmal verstehen muss.

    Der folgende Test ist Punkt 4 aus der Aufzählung oben. Hier ist mir eine Sache wichtig:
    Anstatt das Jacobisymbol (h * 2^k + 1 , D) zu berechnen, kann man reduzieren und zwar:

    h % D statt h

    k % (D-1) statt k


    Für statistische Zwecke lasse ich mir nicht nur das Ergebnis ausgeben, ob n eine Primzahl ist. Sondern auch die Primzahl D, mit der das Jacobisymbol -1 ergibt.

    Sollten alle Primzahl in der vorgelegten List 1 ergeben, so ist die Liste einfach nicht lang genug. Dann gebe -1 zurück.



    Das funktioniert auch soweit schonmal. Dann habe ich mich an die Auswertung gemacht. Ich möchte einen Exponenten k vorgeben und für verschiedene h prüfen, ob h*2^k+1 prim ist oder nicht.

    Dazu prüfe ich mit dem oben beschrieben Verfahren die Tupel (h, k) und lege das Ergebnis als Tupel (h, k, True/False) in einer Liste ab.

    Dann akkumuliere ich die Werte in der Liste und plotte die Verteilung der Primzahlen.

    Da ich schon Kommentare im Code habe, gebe ich diesen einfach mal komplett wieder.

    Bitte einfach schimpfen wenn das nicht korrekt ist ;)

    Es funktioniert auch wunderbar und ich bekomme schöne plots dabei raus. Aber Verbesserungen sind immer wichtig ;)


    Vielen Dank für eure Hilfe!

    Hier einmal das Skript als Datei: statistik_in_einer_datei.py

    Außerdem: die Primzahlliste. Gezippt, da sonst "ungültige Dateiendung" kommt.

    primes_up_to_10_7.zip

  • Zeig mal bitte das gesamte Skript als ganzes, damit man die Zusammenhänge besser erkennen kann! ;)

    Habe ich oben als Spoiler eingefügt und hier nochmal ganz normal ;)


  • Habe ich oben als Spoiler eingefügt und hier nochmal ganz normal ;)

    Sorry, habe ich übersehen!


    Jetzt klick mal bitte noch unter Deinem Beitrag #3 auf "Bearbeiten", dann oben in dem Codeblock auf Quellcode und wähle dort unter Syntax-Hervorhebung Python aus, dann "Speichern" und dann unter dem Beitrag noch "Speichern". Das macht den Code nochmals etwas lesbarer. ;)

  • [...] Zahlen der Form n = h * 2^k + 1, wobei h ungerade und kleiner 2^k ist. Nun kann man prüfen, ob die Zahl n eine Primzahl ist:

    Wenn ich nicht völlig daneben bin, kann n niemals eine Primzahl sein (sofern h nicht 1 ist), denn n ist mindestens durch h und 2^k+1 sowie durch sich selbst und 1 teilbar.

    Oh, man kann hier unliebsame Nutzer blockieren. Wie praktisch!

  • neuernutzer vielleicht bin ich als python greenhorn zu doof, aber ich sehe nicht, dass dein Code wirklich was macht.

    Neben vielen Definitionen von Funktionen liest du eine Datei ein (Zeile 14), und ab Zeile 125 werden ein paar Variablen belegt. Die ganzen Funktionen werden aber niemals aufgerufen. Fehlt da nicht noch ein main?

  • Du meinst h* (2^k + 1).

    Stimmt, das hab ich so interpretiert, aber das stimmt natürlich nicht.

    Was erhoffst du dir denn von der Rechnerei. Meinst du nicht, das haben schon hunderte Leute mit Supercomputern durchgerechnet? Was gedenkst du da mit einem popligen Pi zu erreichen?

    Oh, man kann hier unliebsame Nutzer blockieren. Wie praktisch!

  • neuernutzer vielleicht bin ich als python greenhorn zu doof, aber ich sehe nicht, dass dein Code wirklich was macht.

    Neben vielen Definitionen von Funktionen liest du eine Datei ein (Zeile 14), und ab Zeile 125 werden ein paar Variablen belegt. Die ganzen Funktionen werden aber niemals aufgerufen. Fehlt da nicht noch ein main?

    Davon habe ich keine Ahnung. Ich kompiliere das einfach jedesmal mit anderen Werten und schaue mir die Ergebnisse an. Der Code selbst funktioniert ja.

  • Hallo,


    'time' wird importiert aber nicht genutzt.

    Auf Modulebene (der Code ohne Einrückungen) darf kein ausführbarer Code stehen. Hier werden nur Klassen, Funktionen und Konstanten definiert. Es gibt eine Ausnahme, das ist eine 'if'-Abfrage als Einstiegspunkt in die 'main'-Funktion. Darin sollte dein Programm starten und von dort werden weitere Funktionen aufgerufen, Argumente übergeben und wieder entgegen genommen.


    Gewöhne dir gleich an sprechende Namen zu verwenden und keine Abkürzungen. Das ist in der Mathematik manchmal etwas schwer, aber es ist auch sehr schwer Code zu lesen, der nur aus Namen mit einem Buchstaben besteht.


    Wenn du mit Dateien arbeitest solltest du immer den absoluten Pfad verwenden. Ein Pfad ist aber kein einfacher String. Den ein Pfad hat andere Eigenschaften als ein String. Erstellen kannst du den Pfad mit 'pathlib.Path'.


    Wenn du eine Datei öffnest, solltest du immer das encoding angeben. Es ist nicht garantiert, das automatisch das richtige genommen wird.


    Bei 'range' musst du den 'step' nicht angeben, wenn der 1 sein soll, das ist der default-Wert.


    Wieso belegst du 'h_ober' mit -1 und addierst bei der Benutzung 1 dazu? Dann kannst du doch gleich 'h_ober = 0' schreiben?

    'h_unter', 'h_ober', 'exponenten_unter' und 'exponenten_ober' sollten Konstanten sein.


    An 'plotten' wird 'obergrenze' übergeben, aber nicht benutzt.

    Strings formatiert man aktuell eigentlich mit 'f'-Strings.

    Wenn die 'obergrenze' 0 ist dann kann sie nicht auch größer 2 sein. Das dritte 'if' in 'intervall_prüfen' sollte ein 'elif' sein. Das heißt, wenn das vorherige True war, dann wird 'elif' gar nicht mehr geprüft. Komischer finde ich aber, das egal ob die Obergrenze 0 oder >2 ist, die Rechnung gleich ist. Zu was dann die Unterscheidung?

    Den Sinn hinter 'ausgabe' verstehe ich nicht. Wann setzt man das True? Vielleicht wäre ein sprechenderer Name hier auch hilfreicher.

    'B' ist auch so ein Name, aber nicht nur das er nichts aussagt, an der Stelle wollte ich noch sagen, das man Namen klein_mit_unterstrich schreibt. Ausnahmen sind Konstanten GANZ_GROSS und Klassen in PascalCase-Schreibweise.

    In 'jacobisymbol' kannst du diesen Ausdruck:

    Code
        if a == 2:
            if n % 8 == 1 or n % 8 == 7:
                return 1
            else:
                return -1

    kannst du etwas zusammen rücken:

    Code
        if a == 2:
            return 1 if n % 8 in [1, 7] else -1

    Dann kannst du die 'if' an manchen Stellen wieder durch 'elif' ersetzen.

    'while' braucht keine Klammern.


    In 'bosma' kannst du

    Code
                if pow(int(D), exp, n) == n - 1:
                    return [True, D]
                return [False, D]

    wieder vereinfachen

    Code
    return [True, D] if pow(int(D), exp, n) == n - 1 else [False, D]

    und ein 'elif' einbauen.

    Es ist verwirrend wenn eine Funktion zwei Arten von Rückgabewerte hat. Eine Liste oder eine negative Zahl. Normal gibt eine Funktion immer den gleichen Datentyp zurück.


    'vektor_anlegen' wird nicht benutzt. Man muss nicht alles an einen Namen binden, wenn man den Namen nur benutzt um ihn danach zum Beispiel zurück zu geben. dann kannst du gleich return laenge * [False] schreiben.


    In 'liste_vorbereiten' man will Index so wenig wie möglich benutzen, weil das relativ undurchsichtig ist. Du kannst direkt über den Inhalt der Tuple iterieren:

    Code
        for x_werte, _, y_roh in vektor:
            xwerte.append(x_werte)
            y_rohdaten.append(y_roh

    In 'liste_vorbereiten' würde ich die Werte nicht als Liste zurück geben, sondern einfach zwei Werte und die dann auch so entgegen nehmen und dabei kannst du dann wieder auf den Index verzichten.


    Soviel mal zum Allgemeinen Code. Für mehr Details ist es mir jetzt ehrlich gesagt zu spät und den Zwischenstand von dem hier geschriebenen in Form von Code, reiche ich morgen nach :thumbup:


    Grüße

    Dennis

    🎧 Marylin´s Befehle an meine Junge Seele hör ich in jedem Lied. Heimlich eingegeben, in mein Innenleben, durch den Hard-Rock Beat 🎧

  • Du nutzt aus 'bosma' immer nur den ersten Wert aus der zurückgegebenen Liste, wieso gibst du die anderen dann zurück?

    Und man muss beim Vergleiche mit einem Wahrheitswert nicht extra noch mal den Wahrheitswert hinschreiben. 'if eintrag == True' ist das gleiche wie 'if eintrag'.


    So hier mal der erste ungetestete Zwischenstand:


    Das sind jetzt keine großen Änderungen, aber doch grundlegende Themen.


    Grüße

    Dennis

    🎧 Marylin´s Befehle an meine Junge Seele hör ich in jedem Lied. Heimlich eingegeben, in mein Innenleben, durch den Hard-Rock Beat 🎧

  • neuernutzer Alternativ zu absoluten Pfaden kann man die auch relativ zum Modul angeben. Hinter `__file__` verbirgt sich in jedem Modul der Pfad zu dem Modul (sofern das Modul aus einer Datei geladen wurde).


    Bei `listload()` und `listsave()` kann das `list` aus dem Namen raus, denn `pickle` kann fast alles speichern und laden, nicht nur Listen.


    Beide Funktionen bekommen ein Argument `datei` und haben einen lokalen Namen `file`. Nun ist das aber von der Bedeutung her das gleiche. Die Werte haben aber nicht die gleiche Bedeutung. Das ist verwirrend. `datei` ist gar keine Datei, sondern ein Datei*name*. Also entweder eine Zeichenkette oder ein `Path`-Objekt das den Namen (eventuell inklusive Pfad) repräsentiert. Während man bei einem Wert der eine Datei repräsentiert Methoden wie `read()` oder `write()` und `close()` erwartet.


    Der Hinweis von Dennis89 mit der Kodierung greift hier allerdings nicht. Der ist richtig und zwar für *Textdateien*. Das sind Pickle-Dateien aber nicht, und die werden im Code ja auch korrekt im Binärmodus geöffnet.


    Bei den Exponenten kann man sich den `list()`-Aufruf sparen. `range()`-Objekte haben bereits alle Eigenschaften die der Code benötigt — da kann man drüber iterieren.


    `liste` ist ein zu generischer Name für einen Wert von dem der Leser wissen sollte, dass da Primzahlen drin sind.


    Die Kommentare die erklären was eine Funktion macht, gehören nicht als Kommentar vor die Funktion, sondern als Docstring in die Funktion.


    Bei `intervall_pruefen()` stimmt die Beschreibung nicht. Da steht für alle ungeraden `h`, aber wenn `ausgabe` den Wert `True` hat, werden auch die geraden `h` geprüft. Was stimmt denn nun? Der Code oder die Dokumentation?


    `ausgabe` hat Dennis89 ja bereits angesprochen. Im Code ist dann auch noch mal ein Kommentar der nötig ist, weil das so ein bisschend verwirrend ist. Kann es sein das in beiden Fällen nur die ungeraden `h` bearbeitet werden sollen? Dann steht da fast der gleiche Code doppelt. Der sollte da nur einmal stehen und nur das `print()` sollte bedingt ausgeführt werden.


    Es sieht auch so ein bisschen danach aus, als wenn hier keine selbst gebastelte Lösung, sondern vielleicht Logging verwendet werden sollte.


    `exp` in `bosma()` wird auch berechnet wenn das gar nicht verwendet wird. Das sollte in den entsprechenden ``if``-Zweig wandern.


    Listen sollten für Werte verwendet werden die alle die gleiche Bedeutung haben. Wenn die Bedeutung eines Elements an den Index gekoppelt ist, dann verwendet man Tupel.


    ``int(prime)`` sieht komisch aus, denn Primzahlen sind ja ganze Zahlen, da ist das `int()` etwas verwirrend.


    Zu der -1 als Rückgabewert von `bosma()` hat Dennis89 ja auch schon was gesagt. Die würde ja unweigerlich zu einem `IndexError` führen beim Aufrufer, denn das ``B[0]`` funktioniert bei einer ganzen Zahl ja nicht. Kann es sein, dass hier eigentlich ausgedrückt werden soll, dass dieser Fall nicht eintreten darf in `bosma()`? Dann sollte man das *dort* auch so ausdrücken, entweder mit einem ``assert`` oder mit einer Ausnahme. Nicht mit einem magischen Rückgabewert der an anderer Stelle zu Folgefehlern führt.


    In `jacobisymbol()` ist das Problem wieder: Da sollte am Anfang nicht implizit `None` zurückgegeben werden sondern eine Ausnahme ausgelöst werden wenn da Werte übergeben werden, mit denen man an der Stelle kein sinnvolles Ergebnis berechnen kann.


    Ich lande dann als Zwischenstand hier (ungetestet):

    „Eat the rich — the poor are full of preservatives.“ — Rebecca ”Becky” Connor, The Connors

  • Post by neuernutzer ().

    The post was deleted, no further information is available.
  • Guten Morgen,


    ich gehe nach und nach die Punkte aus euren Beiträgen durch. Fange ich mal an mit dem Beitrag von Dennis89


    Quote

    Gewöhne dir gleich an sprechende Namen zu verwenden und keine Abkürzungen. Das ist in der Mathematik manchmal etwas schwer, aber es ist auch sehr schwer Code zu lesen, der nur aus Namen mit einem Buchstaben besteht.

    Ich verstehe. Du hast Recht, das ist vor allem in der Mathematik schwer, denn in meiner Arbeit heißen die Variablen eben h und k. Das ist auch durchgehend so, sodass man als Leser nahezu immer weiß "h ist der Koeffizient, k ist der Exponent".


    Quote

    Wenn du eine Datei öffnest, solltest du immer das encoding angeben. Es ist nicht garantiert, das automatisch das richtige genommen wird.

    Dann bekomme ich "ValueError: binary mode doesn't take an encoding argument".

    Wieso belegst du 'h_ober' mit -1 und addierst bei der Benutzung 1 dazu? Dann kannst du doch gleich 'h_ober = 0' schreiben?

    Der Bosma-Test funktioniert für alle ungeraden h zwischen 1 und 2^k.
    Mein Gedanke war: Wenn ich alle diese h prüfen möchte, gebe ich 0 als Obergrenze an. Wenn ich eine spezielle Obergrenze haben möchte, kann ich sie manuell eintragen.


    Quote

    Es ist verwirrend wenn eine Funktion zwei Arten von Rückgabewerte hat. Eine Liste oder eine negative Zahl. Normal gibt eine Funktion immer den gleichen Datentyp zurück

    Der HIntergrund ist, dass ich mit der Funktion bosma verschiedene Statistiken erstellen möchte. Einmal die Auswertung der Primzahlen (wie aktuell), ein anderes mal die benutzten Primzahlen D.


    Wenn die 'obergrenze' 0 ist dann kann sie nicht auch größer 2 sein. Das dritte 'if' in 'intervall_prüfen' sollte ein 'elif' sein. Das heißt, wenn das vorherige True war, dann wird 'elif' gar nicht mehr geprüft. Komischer finde ich aber, das egal ob die Obergrenze 0 oder >2 ist, die Rechnung gleich ist. Zu was dann die Unterscheidung?

    Das habe ich nun geändert:

    Code
        if obergrenze == 0 or obergrenze >= 2**exponent:
            obergrenze = 2**exponent - 1
            print(f"Obergrenze auf {obergrenze} angepasst")


    Den Sinn hinter 'ausgabe' verstehe ich nicht. Wann setzt man das True? Vielleicht wäre ein sprechenderer Name hier auch hilfreicher.

    Wenn ausgabe = True ist, wird mir in jedem Step der aktuelle Fortschritt in Prozent angegeben. Ich habe das in einen eigenen Block gepackt, da ich sonst jedesmal die if-Abfrage gehabt hätte, ob ausgabe auf True steht.

  • Hallo,

    Sorry, habe ich übersehen!


    Jetzt klick mal bitte noch unter Deinem Beitrag #3 auf "Bearbeiten", dann oben in dem Codeblock auf Quellcode und wähle dort unter Syntax-Hervorhebung Python aus, dann "Speichern" und dann unter dem Beitrag noch "Speichern". Das macht den Code nochmals etwas lesbarer. ;)

    Entschuldige, den Beitrag hatte ich übersehen. Das habe ich nun gemacht.