Allgemein: Codeoptimierung

Code-Stücke können hier veröffentlicht werden.
Antworten
sirdonat
User
Beiträge: 12
Registriert: Dienstag 10. Dezember 2013, 09:14

Hallo,
ich habe eine Look-Up-Table geschrieben, die Positionen mit Informationen versieht, die ein Roboter theoretisch anfahren kann. Der Roboter kann sich 6-dimensional im Raum bewegen (3 translatorsche Freiheitsgrade X,Y,Z und drei rotatorische Xe, Ye, Ze). Es handelt sich aslo um ein 6 dimensionales Array. Die Informationen können sein: gültige Position (="0") oder ungegültige Postion (eine Zahl<32678, die aus addierten Binärzahlen besteht, die codiert die Gründe für das Nicht-Erreichen dieser Position wiedergeben.
Da die Datenmenge der look-up-table möglichst gering sein soll, habe ich beschlossen, Bereiche und nicht genaue Positionen mit Werten zu versehen.
Der unten stehende Code funktioniert fehlerfrei, nur ist er extrem unprofessionel, und ich möchte euch bitten mir zu helfen ihn effizienter und professioner zu gestalten.
Der Code soll die Lookuptable auslesen und sagen, ob die Input-Position gültig oder ungültig ist.
Die Look-up-table besteht aus zwei Teilen: der rough_matrix und der detailed_matrix. Die rough_matrix beschreibt Bereiche, die man sich als 6-dimensionale Würfel mit einer Kantenlänge von 8 Einheiten vorstellen kann. Jeder dieser Würfel ist entweder komplett mit Gültigen Positionen im Inneren versehen(Eintrag "0"), ungültig("addierte Binärzahl<32768) oder liegt im Grenzbereich und hat also im Inneren gültige und ungültige Positionen. Wenn, dass der fall ist, wird dieser Würfel mit einer Adresse deklariert (Eine Zahl>=32768), die auf eine Stelle in der detailed_matrix verweist. Die detailed_matrix ist ein 7-dimensionales array, da sie noch eine Spalte für die Adresse benötigt. Unter jeder Adresse wird der 8er Würfel in 64 kleiner Würfel aufgeteilt, die eine Kantenlänge von 4 Einheiten haben, die wiederrum gültig/ungültig oder im grenzbereich liegen. 4er Würfel im Grenzbereich werden wiederum mit Adressen deklariert, die auf eine Stelle in der detailed_matrix verweisen, die den 4er Würfel in 64 kleiner Wüfel mit 2 Einheiten kantenlänge unterteilen und die neue deklarieren.
Das klingt alles sehr abstrakt, ich weiß. Hoffe ihr könnt mir trotzdem weiterhelfen. Danke!



Code: Alles auswählen

import vtk



def check_position():
	xs = x.get()
	ys = y.get()
	zs = z.get()
	xes = xe.get()
	yes = ye.get()
	zes = ze.get()
	input = (float(xs), float(ys), float(zs), float(xes), float(yes), float(zes))
#definierte Bereiche (von, bis)
	X = (-16, 17)
	Y = (-8, 57)
	Z = (-16, 17)
	Xe = (-64, 65)
	Ye = (-16, 17)
	Ze = (-16, 49)

	#python listen statt numpy arrays, damit index() funktioniert
	x_array = numpy.arange(X[0], X[1], 8)
	x_list = x_array.tolist()
	y_array = numpy.arange(Y[0], Y[1], 8)
	y_list = y_array.tolist()
	z_array = numpy.arange(Z[0], Z[1], 8)
	z_list = z_array.tolist()
	xe_array = numpy.arange (Xe[0], Xe[1], 8)
	xe_list = xe_array.tolist()
	ye_array = numpy.arange(Ye[0], Ye[1], 8)
	ye_list = ye_array.tolist()
	ze_array = numpy.arange(Ze[0], Ze[1], 8)
	ze_list = ze_array.tolist()


	#inputzahlen auf durch 8 teilbare zahl abrunden, da diese 8er Wuerfel deklarieren
	input_floored_8 = []
	for i in range(0, len(input)):
		input_floored_8.append(8 * math.floor (input[i] / 8))
	print input_floored_8

	#index in listen suchen
	xindex = x_list.index(input_floored_8[0])
	yindex = y_list.index(input_floored_8[1])
	zindex = z_list.index(input_floored_8[2])
	xeindex = xe_list.index(input_floored_8[3])
	yeindex = ye_list.index(input_floored_8[4])
	zeindex = ze_list.index(input_floored_8[5])

	if rough_matrix[xindex][yindex][zindex][xeindex][yeindex][zeindex] < 32768:
		if rough_matrix[xindex][yindex][zindex][xeindex][yeindex][zeindex] == 0:
			print input, "is reachable"
		else:
			print input, "is not reachable, Reason: ", rough_matrix[xindex][yindex][zindex][xeindex][yeindex][zeindex]

	else:
		print "4mm/4degree solution necessary"
		address = int(rough_matrix[xindex][yindex][zindex][xeindex][yeindex][zeindex] - 32768)
		input_floored_4 = []
		for i in range(0, len(input)):
			input_floored_4.append(4 * math.floor (input[i] / 4))
		print "4mm/4degree coordinates: ", input_floored_4

		if input_floored_4[0] == input_floored_8[0]:
			x0 = 0
		else:
			x0 = 1

		if input_floored_4[1] == input_floored_8[1]:
			y0 = 0
		else:
			y0 = 1

		if input_floored_4[2] == input_floored_8[2]:
			z0 = 0
		else:
			z0 = 1

		if input_floored_4[3] == input_floored_8[3]:
			xe0 = 0
		else:
			xe0 = 1

		if input_floored_4[4] == input_floored_8[4]:
			ye0 = 0
		else:
			ye0 = 1

		if input_floored_4[5] == input_floored_8[5]:
			ze0 = 0
		else:
			ze0 = 1

		if detailed_matrix[address][x0][y0][z0][xe0][ye0][ze0] < 32768:
			if detailed_matrix[address][x0][y0][z0][xe0][ye0][ze0] == 0:
				print input, "is reachable"
			else:
				print input, "is not reachable, Reason: ", detailed_matrix[address][x0][y0][z0][xe0][ye0][ze0]
		else:
			print "2mm/2degree solution necessary!"
			address = int(detailed_matrix[address][x0][y0][z0][xe0][ye0][ze0] - 32768)
			input_floored_2 = []
			for i in range(0, len(input)):
				input_floored_2.append(2 * math.floor (input[i] / 2))
			print "2mm/2degree cube-coordinates: ", input_floored_2

			if input_floored_2[0] == input_floored_4[0]:
				x0 = 0
			else:
				x0 = 1

			if input_floored_2[1] == input_floored_4[1]:
				y0 = 0
			else:
				y0 = 1

			if input_floored_2[2] == input_floored_4[2]:
				z0 = 0
			else:
				z0 = 1

			if input_floored_2[3] == input_floored_4[3]:
				xe0 = 0
			else:
				xe0 = 1

			if input_floored_2[4] == input_floored_4[4]:
				ye0 = 0
			else:
				ye0 = 1

			if input_floored_2[5] == input_floored_4[5]:
				ze0 = 0
			else:
				ze0 = 1
			if detailed_matrix[address][x0][y0][z0][xe0][ye0][ze0] == 0:
				print input, "is reachable"
			else:
				print input, "is not reachable, Reason: ", detailed_matrix[address][x0][y0][z0][xe0][ye0][ze0]
	print "_____________________________________"



Sirius3
User
Beiträge: 17747
Registriert: Sonntag 21. Oktober 2012, 17:20

@sirdonat: zuerst solltest Du einen lauffähigen Code posten und alle globalen Variablen eleminieren.
Dann kannst Du Dir ein Tutorial zu numpy anschauen, und wie man damit rechnet.
Dann können wir den Code nochmal anschauen und Verbesserungsvorschläge machen.
BlackJack

@sirdonat: Ergänzend zu Sirius3: ``for i in range(len(sequence)):`` nur um dann mit `i` als Index auf `sequence` zuzugreifen ist in Python ein „anti pattern”. Man kann in einer Schleife *direkt* über die Elemente von iterierbaren Objekten gehen, ohne einen Umweg über einen Index.

Sowohl in der Beschreibung als auch im Quelltext scheint sich so ziemlich der selbe Vorgang auf verschiedenen Arrays zu wiederholen, das kann man in eine allgemeinere Funktion heraus ziehen. Und auch innerhalb so eines Schrittes gibt es sehr viel Quelltext der nach kopieren und einfügen aussieht.
sirdonat
User
Beiträge: 12
Registriert: Dienstag 10. Dezember 2013, 09:14

@Sirius3: Was verstehst du unter einem lauffähigen Code und Eliminierung aller globalen Variablen? Ich kann ja nicht die rough_matrix und die detailed_matrix, die in dieser Funktion ausgelesen werden, hochladen...
Vielleicht können wir ja mal den Code zeilenweise durchgehen:

Z.B.:

Code: Alles auswählen

	X = (-16, 17)
	Y = (-8, 57)
	Z = (-16, 17)
	Xe = (-64, 65)
	Ye = (-16, 17)
	Ze = (-16, 49)

	#python listen statt numpy arrays, damit index() funktioniert
	x_array = numpy.arange(X[0], X[1], 8)
	x_list = x_array.tolist()
	y_array = numpy.arange(Y[0], Y[1], 8)
	y_list = y_array.tolist()
	z_array = numpy.arange(Z[0], Z[1], 8)
	z_list = z_array.tolist()
	xe_array = numpy.arange (Xe[0], Xe[1], 8)
	xe_list = xe_array.tolist()
	ye_array = numpy.arange(Ye[0], Ye[1], 8)
	ye_list = ye_array.tolist()
	ze_array = numpy.arange(Ze[0], Ze[1], 8)
	ze_list = ze_array.tolist()
wieder sehr viel copy/paste. In diesem Teil werden Listen erstellt, deren Indizes mit denen der rough_matrix übereinstimmen. Wie würde ein professioneller Python-Programmierer diese Listen erstellen?
Also wie gesagt, mein Code funktioniert, ist halt nur schlecht geschrieben. Wenn es keinen Sinn macht, auf diese Weise den Code durchzugehen, schreibt es bitte.
@BlackJack bzgl. for i in range(len(sequence)) am konkreten Beispiel:

Code: Alles auswählen

	input_floored_8 = []
	for i in range(0, len(input)):
		input_floored_8.append(8 * math.floor (input[i] / 8))
	print input_floored_8
besser wäre:

Code: Alles auswählen

	input_floored_8 = []
	for i in input:
		input_floored_8.append(8 * math.floor (i/ 8))
	print input_floored_8
???
Benutzeravatar
sparrow
User
Beiträge: 4193
Registriert: Freitag 17. April 2009, 10:28

sirdonat hat geschrieben:@Sirius3: Was verstehst du unter einem lauffähigen Code und Eliminierung aller globalen Variablen? Ich kann ja nicht die rough_matrix und die detailed_matrix, die in dieser Funktion ausgelesen werden, hochladen...
Dein Code, speziell die hier gezeigte Funktion, verwendet Variablen, aus dem globalen Namensraum. Das ist nicht nur in Python verpöhnt und kein sauberer Stil. Es geht bei Funktionen darum Teilprobleme zu lösen, damit der Code hinterher noch wartbar bleibt und sich leichter lesen und testen lässt.

Du müsstest die Funktion also so anpassen, dass die benötigten Parameter beim Aufruf übergeben werden. Das wäre der erste Schritt. Auch die entsprechende Matrix kannst du so übergeben.
Das kapselt dann schon einmal ein Teilproblem (man müsste dann weiter gehen was noch gekapselt werden muss), macht die Funktion übersichtlicher und lässt sich im Zweifelsfall auch schneller Testen. Eine entsprechende Matrix könnte man zum Test mit Zufallszahlen errechnen lassen, damit die entsprechende Zugriffe funktionieren.

sirdonat hat geschrieben: besser wäre:

Code: Alles auswählen

	input_floored_8 = []
	for i in input:
		input_floored_8.append(8 * math.floor (i/ 8))
	print input_floored_8
???
Das wäre ein Schritt weiter, aber das schreit regelrecht nach einer List Comprehension:

Code: Alles auswählen

input_floored_8 = [8*math.floor(i/8) for i in input]
BlackJack

@sirdonat: Lauffähig so dass man ihn tatsächlich mal ausprobieren kann. Mit Eliminierung der globalen Variablen ist gemeint, dass man in einer Funktion keine Daten verwenden sollte, die nicht als Argument übergeben wurden. Ausgenommen Konstanten. Die `rough_matrix` und die `detailed_matrix` sollten also zum Beispiel übergeben werden und nicht einfach so aus dem „nichts” kommen.

Wenn Du die Daten nicht hochladen kannst, könnte man zumindest eine kleine Funktion schreiben mit der man Beispieldaten erzeugen kann. Wenn möglich welche die alle interessanten Fälle abdecken. So etwas wäre aus „professioneller” Sicht sowieso interessant, weil Du dann auch für Dich Unit-Tests schreiben kannst.

Den Quelltextschnippsel könnte man in das hier umsetzen:

Code: Alles auswählen

    (
        x_values,
        y_values,
        z_values,
        xe_values,
        ye_values,
        ze_values,
    ) = (
        np.arange(a, b, 8).tolist()
        for a, b in [
            (-16, 17), (-8, 57), (-16, 17),(-64, 65), (-16, 17), (-16, 49)
        ]
    )
Weder `*_list` noch `*_values` ist nicht besonders aussagekräftig, das sollte man vielleicht besser benennen. Wobei mir das sowieso etwas komisch vorkommt vom Vorgehen, denn zumindest vom Gefühl her sollte man das Ergebnis von dem `index()`-Aufruf *direkt* berechnen können, ohne danach in einer Liste linear zu suchen.

Wo kommen eigentlich die Zahlen für die Grenzen her? Das sieht ein wenig „magisch” aus.

Zur Schleife: Ja, aber noch besser wäre eine „list comprehension”:

Code: Alles auswählen

    input_floored_8 = [(x // 8) * 8 for x in input]
    print input_floored_8
Die Antwort hatte sparrow mittlerweile ja auch schon gegeben, aber ich habe hier noch die Ganzzahldivision mit ``//`` ins Spiel gebracht. :-)
sirdonat
User
Beiträge: 12
Registriert: Dienstag 10. Dezember 2013, 09:14

super, das hilft mir schon einmal sehr weiter. Vielen Dank euch beiden!!
Also ich werde die Dinge mal einbauen und morgen einen lauffähigen Code mit euren Verbesserungen hochladen.
sirdonat
User
Beiträge: 12
Registriert: Dienstag 10. Dezember 2013, 09:14

BlackJack hat geschrieben: Wo kommen eigentlich die Zahlen für die Grenzen her? Das sieht ein wenig „magisch” aus.
Das sind Koordinaten, in denen der Roboter sich bewegt(also in x-Richtung von -16mm bis +16mm , Drehung um die Z-Achse von -16° bis +48°, etc).
Wenn man also als input(0,2,0,19.2123,0,-7) bekommt, soll die Funktion überprüfen, ob die Postion X=0mm,Y=2mm,Z=0mm bei Drehungen um x-Achse=19,2123°, um die y-Achse=0°, um die z-Achse=-7° für den Roboter erlaubt ist, oder nicht.
BlackJack

@sirdonat: Das ist also wirklich ein wenig magisch das das da so hart kodiert mitten in einer Funktion steht. Dagegen müsste man die Eingabedaten dann vielleicht auch prüfen, oder?

Was mir gerade noch auffällt: Nach Möglichkeit Benutzerinteraktion von der Programmlogik trennen. Also Zahlenwerte übergeben und keine GUI-Widget-Objekte und Rückgabewerte statt direkt ``print``. Wenn von den ``print``\s welche zur Fehlersuche drin sind, würde ich das `logging`-Modul stattdessen vorschlagen. Da kann man solche Debuggingausgaben dann auch einfacher ausschalten und bei Bedarf wieder aktivieren.
Benutzeravatar
sparrow
User
Beiträge: 4193
Registriert: Freitag 17. April 2009, 10:28

BlackJack hat geschrieben:Wobei mir das sowieso etwas komisch vorkommt vom Vorgehen, denn zumindest vom Gefühl her sollte man das Ergebnis von dem `index()`-Aufruf *direkt* berechnen können, ohne danach in einer Liste linear zu suchen.
Das Berechnen ist ja relativ einfach. Ich habe in "old_version" mal den numpy-Kram durch das normale range ersetzt. Ich weiß nicht ob numpy mit anschließender Konvertierung in eine Liste schneller wäre, aber ich habe kein numpy installiert um es zu testen.
Ansonsten wird in old_version all das gemacht, was sirdonat auch tut.

new_version ist dann ohne Indexe und läuft rund 33% schneller als die Variante in old_version.

@sirdonaut: Ich habe die Namensgebung grob so gelassen, damit du dich zurecht findest - da ist sicherlich noch Potential. Die Namen der Dimensionen für die Reihenfolge (und die Prüfung ob auch alle richtig heißen ;) ) und die possible_ranges wären in meinen Augen Konstanten die ich man durch GROSSBUCHSTABEN bei der Namensgebung kennzeichnet. Da könnte man dann schauen ob man diese Konstanten nicht lieber an einer Zentralen Stelle definiert. "input" ist keine gute Idee für eine Variable, weil damit die interne Funktion input() überschrieben wird und die meisten Programmierer wohl mit einer Eingabe an dieser Stelle rechnen.


Und ja, ich mag Dictionaries in diesem Fall lieber als Listen, weil ich nach 6 Sekunden vergessen habe welcher Wert in der Liste nun welche Koordinate angibt. Die Änderungen für Listen wären aber Minimal.

Code: Alles auswählen

def old_version(input):

    X = (-16, 17)
    Y = (-8, 57)
    Z = (-16, 17)
    Xe = (-64, 65)
    Ye = (-16, 17)
    Ze = (-16, 49)

    x_list = range(X[0], X[1], 8)
    y_list = range(Y[0], Y[1], 8)
    z_list = range(Z[0], Z[1], 8)
    xe_list = range(Xe[0], Xe[1], 8)
    ye_list = range(Ye[0], Ye[1], 8)
    ze_list = range(Ze[0], Ze[1], 8)

    input_floored_8 = [(x // 8) * 8 for x in input]

    xindex = x_list.index(input_floored_8[0])
    yindex = y_list.index(input_floored_8[1])
    zindex = z_list.index(input_floored_8[2])
    xeindex = xe_list.index(input_floored_8[3])
    yeindex = ye_list.index(input_floored_8[4])
    zeindex = ze_list.index(input_floored_8[5])

    return xindex, yindex, zindex, xeindex, yeindex, zeindex



def new_version(input):

    def get_index(value, possible_range, step):
        if possible_range[0] <= value <= possible_range[1]:
            return (value - possible_range[0]) // step
        else:
            raise ValueError("%i not in possible range" % (value,))

    dimensions = ("x", "y", "z", "xe", "ye", "ze")
    possible_ranges = {"x": (-16, 17), "y": (-8, 57), "z": (-16, 17),
                      "xe": (-64, 65), "ye": (-16, 17), "ze": (-16, 49)}

    indexes = [get_index(input[dimension], possible_ranges[dimension], 8)
               for dimension in dimensions]
    return indexes


if __name__ == "__main__":
    input = (3.1, -4.1, 8.5, -3.1, 4.7, 8.6)
    print old_version(input)
    input = {"x": 3.1, "y": -4.1, "z": 8.5, "xe": -3.1, "ye": 4.7, "ze": 8.6}
    print new_version(input)
sirdonat
User
Beiträge: 12
Registriert: Dienstag 10. Dezember 2013, 09:14

@sparrow: Vielen Dank. Besonders gut hat mir die Berechnung der Indizes, ansttatt sie zu suchen, gefallen.Ich habe deine funktion aufgesplittet, da ich den Teil, der für die Abrundung zuständig ist, mehrmals benötige. Ich bin auch erstmal bei listen anstatt dictionaries geblieben, da ich eine Liste mit der zu überprüfenden Position als globale Variabel in der hier besprochenden Funktion überwiesen bekomme. Die Liste input habe ich in position geändert.
@BlackJack: Die Eingabedatenüberprüfung habe ich so eingefügt, wie es sparrow vorgeschlagen hat. Wegen des "logging"Moduls,muss ich mich noch schlau machen.
Zur weitern Funktionsweise des Codes, habe ich ein Bild eingefügt:
Bild
Der Einfachheit halber kann sich der hexapod hier nur 2dimensional bewegen. Das Bild oben stellt den gesamten Bereich in 2d-Würfel(=Quadrate) mit Kantenlänge 8 da. Der Rote Bereich ist für den roboter nicht erreichbar. Der Schwarze Punkt stellt die zu überprüfende Position dar. Er liegt im Grenzbereich und hat hat die position (5.5,0.8), abgerundet(0,0), was den Indexen [2,2] entspricht. Also steht wegen des Grenzbereichs in der rough_matrix an Stelle [2][2] zum Beispiel "32786" , eine Adresse. Ich gehe also in die detailed_matrix an der Stelle 32786-32786=0, wo 4 Würfel mit Kantenlänge 4 definiert sind. (Bei 6 Dimensionen wären es 64 Würfel). Die detailed_matrix ist so aufgebaut [adresse-32768][x][y] wobei x und y nur die Werte 0 oder 1 annehmen können. Die Position liegt hier bei [0][1][0] und im grenzbereich, also gibt es für die Position auch eine 2mm Auflösung und eine neue Adresse steht an dieser Stelle in der detailed_matrix( detailed_matrix[0][1][0]=32787). Dann suche ich an dieser Stelle der detailedmatrix nach dem richtigen x,y (hier (0,0) und erhalte die Information, dass die Eingabeposition nicht erreichbar ist.
Im folgenden mein neuer lauffähiger Code. Ich würde gerne die ganzen if/else copy/paste verbessern.

Code: Alles auswählen

import numpy


def get_indexes(input_floored_8):
	possible_ranges = [[-16, 17], [-8, 57], [-16, 17], [-64, 65], [-16, 17], [-16, 49]]

	indexes = []
	for koordinate in range(0, 6):
		if possible_ranges[koordinate][0] <= input_floored_8[koordinate] <= possible_ranges[koordinate][1]:
			indexes.append((input_floored_8[koordinate] - possible_ranges[koordinate][0]) // 8)
		else:
			raise ValueError("%i not in possible range" % (input_floored_8(koordinate),))
	return indexes

def get_floored_input(position, resolution):
	input_floored = [(x // resolution) * resolution for x in position]
	return input_floored

if __name__ == "__main__":
	#beliebiges input
	position = [3.1, 4.1, 8.5, -3.1, 4.7, 8.6]
	rough_matrix = numpy.load("link/rough_matrix.npy")
	detailed_matrix = numpy.load("link/detailed_matrix.npy")
	#indexe zum Auselesn der rough-matrix finden
	input_floored_8 = get_floored_input(position=position, resolution=8)
	indexes = get_indexes(input_floored_8)

	if rough_matrix[indexes[0]][indexes[1]][indexes[2]][indexes[3]][indexes[4]][indexes[5]] < 32768:
		if rough_matrix[indexes[0]][indexes[1]][indexes[2]][indexes[3]][indexes[4]][indexes[5]] == 0:
			print position, "is reachable"
			#eigentlich 'return True'
		else:
			print position, "is not reachable, Reason: ", rough_matrix[indexes[0]][indexes[1]][indexes[2]][indexes[3]][indexes[4]][indexes[5]]
			#eigentlich 'return False'

	else:
		address = int(rough_matrix[indexes[0]][indexes[1]][indexes[2]][indexes[3]][indexes[4]][indexes[5]] - 32768)
		input_floored_4 = get_floored_input(position=position, resolution=4)


		if input_floored_4[0] == input_floored_8[0]:
			x0 = 0
		else:
			x0 = 1

		if input_floored_4[1] == input_floored_8[1]:
			y0 = 0
		else:
			y0 = 1

		if input_floored_4[2] == input_floored_8[2]:
			z0 = 0
		else:
			z0 = 1

		if input_floored_4[3] == input_floored_8[3]:
			xe0 = 0
		else:
			xe0 = 1

		if input_floored_4[4] == input_floored_8[4]:
			ye0 = 0
		else:
			ye0 = 1

		if input_floored_4[5] == input_floored_8[5]:
			ze0 = 0
		else:
			ze0 = 1

		if detailed_matrix[address][x0][y0][z0][xe0][ye0][ze0] < 32768:
			if detailed_matrix[address][x0][y0][z0][xe0][ye0][ze0] == 0:
				print position, "is reachable"

			else:
				print position, "is not reachable, Reason: ", detailed_matrix[address][x0][y0][z0][xe0][ye0][ze0]
				#eigentlich 'return False'
		else:

			address = int(detailed_matrix[address][x0][y0][z0][xe0][ye0][ze0] - 32768)
			input_floored_2 = get_floored_input(position=position, resolution=2)

			if input_floored_2[0] == input_floored_4[0]:
				x0 = 0
			else:
				x0 = 1

			if input_floored_2[1] == input_floored_4[1]:
				y0 = 0
			else:
				y0 = 1

			if input_floored_2[2] == input_floored_4[2]:
				z0 = 0
			else:
				z0 = 1

			if input_floored_2[3] == input_floored_4[3]:
				xe0 = 0
			else:
				xe0 = 1

			if input_floored_2[4] == input_floored_4[4]:
				ye0 = 0
			else:
				ye0 = 1

			if input_floored_2[5] == input_floored_4[5]:
				ze0 = 0
			else:
				ze0 = 1
			if detailed_matrix[address][x0][y0][z0][xe0][ye0][ze0] == 0:
				print position, "is reachable"
				#eigentlich 'return True'
			else:
				print position, "is not reachable, Reason: ", detailed_matrix[address][x0][y0][z0][xe0][ye0][ze0]
				#eigentlich 'return False'

BlackJack

@sirdonat: Erst einmal würde ich den Quelltext unter der ``if __name__…``-Abfrage in eine Funktion stecken. Dann besteht gar nicht erst die Gefahr, dass man in irgendwelchen anderen Funktionen auf globale Daten zugreift.

In der `get_indexes()` hast Du es schon wieder gemacht: Über eine Laufvariable als Index auf Liste zugegriffen über deren Elemente du eigentlich iteriereren wolltest. Und diesmal ist die Länge dieser Liste auch noch händisch als feste Zahl eingetragen worden. Da Du über zwei Objekte „parallel” iterieren möchtest müsste man hier die `zip()`- oder die `itertools.izip()`-Funktion verwenden.

Im Fehlerfall ist übrigens ein Fehler in der Funktion, weil Du dann versuchst `input_floored_8` mit `koordinate` aufzurufen, statt einen Indexzugriff zu machen.

Mit diesen elend langen verschachtelten Zugriffen sollte man irgend etwas machen. Ein Anfang wäre nicht so oft den *selben* Zugriff in den Quelltext schreiben. Auf `rough_matrix` wird viermal zugegriffen, aber immer mit den gleichen Indexwerten. Bei `detailed_matrix` sieht es ähnlich aus.

Die nächste Vereinfachung wäre es eine Funktion zu schreiben die eine Matrix und eine Liste mit Indexwerten bekommt und die nacheinander anwendet, so dass man nicht selber so einen furchtbar langen Zugriff im Quelltext stehen hat.

Anstelle von `True` und `False` würde ich einfach die Bitmaske mit den Gründen zurück geben. Daraus kann der Aufrufer ganz einfach ablesen ob die Position/Ausrichtung erlaubt ist, und wenn gewünscht auch noch weiter analysieren warum nicht wenn das Ergebnis „negativ” ist. Damit würde man selbst wenn man wie jetzt einfach nur ``print``-Ausgaben macht, Quelltext einsprachen der jetzt dreimal fast identisch in der Funktion steht. Die Position und die Matrix-Objekte laden gehört damit auch nicht in diese Funktion sondern in einen Aufrufer, der das dann übergibt.

Bei den vielen ``if``/``else``-Abfragen wo dann eine 0 oder 1 zugiesen wird bietet sich doch wieder eine Schleife an um eine Liste mit Indexen zu erstellen. Da kann man auch ausnutzen das man `True` und `False` in die Zahlen 1 und 0 umwandeln kann.

Ich lande dann bei dem hier (ungetestet):

Code: Alles auswählen

from itertools import izip
import numpy


def get_indexes(input_floored_8):
    possible_ranges = [
        [-16, 17], [-8, 57], [-16, 17], [-64, 65], [-16, 17], [-16, 49]
    ]
    indexes = []
    for (lower, upper), value in izip(possible_ranges, input_floored_8):
        if lower <= value <= upper:
            indexes.append((value - lower) // 8)
        else:
            raise ValueError('{0!r} not in possible range'.format(value))
    return indexes


def get_floored_input(position, resolution):
    return [(x // resolution) * resolution for x in position]


def get_element_by_indices(matrix, indices):
    result = matrix
    for i in indices:
        result = result[i]
    return result


def get_reason_for_position(position, rough_matrix, detailed_matrix):
    input_floored_8 = get_floored_input(position, 8)
    rough_value = get_element_by_indices(
        rough_matrix, get_indexes(input_floored_8)
    )
    if rough_value < 32768:
        return rough_value
    else:
        address = rough_value - 32768
        input_floored_4 = get_floored_input(position, 4)
        indexes = [
            int(a != b) for a, b in izip(input_floored_4, input_floored_8)
        ]
        detailed_value = get_element_by_indices(
            detailed_matrix[address], indexes
        )
        if detailed_value < 32768:
            return detailed_value
        else:
            address = detailed_value - 32768
            input_floored_2 = get_floored_input(position, 2)
            indexes = [
                int(a != b) for a, b in izip(input_floored_2, input_floored_4)
            ]
            return get_element_by_indices(detailed_matrix[address], indexes)


def main():
    #beliebiges input
    position = [3.1, 4.1, 8.5, -3.1, 4.7, 8.6]
    rough_matrix = numpy.load('link/rough_matrix.npy')
    detailed_matrix = numpy.load('link/detailed_matrix.npy')

    reason = get_reason_for_position(position, rough_matrix, detailed_matrix)
    if reason:
        print position, 'is not reachable, Reason: ', reason
    else:
        print position, 'is reachable'


if __name__ == '__main__':
    main()
Die 32768 würde ich vielleicht noch als Konstante heraus ziehen und berechnen statt hinzuschreiben, und eigentlich würde ich auch Bitverknüpfungen besser finden statt ``<`` und ``-``, weil das dem Leser besser Vermittelt das es um Bits geht.
sirdonat
User
Beiträge: 12
Registriert: Dienstag 10. Dezember 2013, 09:14

Hi BlackJack,
der Code läuft fehlerfrei und ich werde ihn so übernehmen. Ganz vielen Dank für deine großartige Hilfe.
BlackJack hat geschrieben: In der `get_indexes()` hast Du es schon wieder gemacht: Über eine Laufvariable als Index auf Liste zugegriffen über deren Elemente du eigentlich iteriereren wolltest. Und diesmal ist die Länge dieser Liste auch noch händisch als feste Zahl eingetragen worden. Da Du über zwei Objekte „parallel” iterieren möchtest müsste man hier die `zip()`- oder die `itertools.izip()`-Funktion verwenden.

Im Fehlerfall ist übrigens ein Fehler in der Funktion, weil Du dann versuchst `input_floored_8` mit `koordinate` aufzurufen, statt einen Indexzugriff zu machen.
ich habe es zuerst zwar mit list-comprehension versucht, aber da kamen dann immer krumme Werte für die Laufvariabel 'koordinate' heraus, der Grund dafür steht oben.

Eine Frage habe ich noch: Das numpy.load() dauert ziemlich lange, da die dateien um 400Mb groß sind. Es macht Sinn, dass der hier behandelte Funktion (check_postion) die detailed_matrix und die rough_matrix schon mitgegeben wird, da im Betrieb des Roboters mehrmals pro Sekunde auf diese Funktion zugegriffen wird. Ich will allerdings vermeiden, dass der Arbeitsspeicher mit den Dateien belastet wird. Kann man diese Dateien auch auslesen, ohne sie in den Arbeitsspeicher zu laden? Und: Ist es überhaupt sinnvoll die detailed_matrix und die rough_matrix als numpy-array zu speichern?
Grüße!
sirdonat
User
Beiträge: 12
Registriert: Dienstag 10. Dezember 2013, 09:14

BlackJack hat geschrieben: Die 32768 würde ich vielleicht noch als Konstante heraus ziehen und berechnen statt hinzuschreiben, und eigentlich würde ich auch Bitverknüpfungen besser finden statt ``<`` und ``-``, weil das dem Leser besser Vermittelt das es um Bits geht.
Kannst du mal ein beispiel für Bitverknüpfungen geben?
also z.B 8<10 entspricht 2^3<2^1+2^3 ???
und statt 32768 besser 2^15????
BlackJack

@sirdonat: Schau Dir mal die Dokumentation zu `numpy.load()` an. Du suchst ganz offenbar das `mmap_mode`-Argument. :-)

Also für die Datenstruktur wie sie jetzt gewählt wurde macht ein Numpy-Array aus Speicherplatzsicht schon sinn. Die gleichen Daten als Listen wäre bei weitem nicht so kompakt.

Ob die Datenstruktur selbst sinnvoll ist, keine Ahnung. Da fehlt mir zum einen die Erfahrung mit dieser Art von Daten und selbst wenn man die hätte währen wahrscheinlich noch Informationen über die Anzahl und Verteilung und vielleicht sonstiger Eigenheiten der Daten nötig. Vielleicht wäre ein Octree ja effizienter. Oder vielleicht auch nicht. :-)

Zu den Bitoperationen:

Code: Alles auswählen

ADDRESS_FLAG = 1 << 15
...
    if rough_value & ADDRESS_FLAG == 0:
        return rough_value
    else:
        address = rough_value & ~ADDRESS_FLAG
...
Jetzt sieht man deutlicher das ein Bit abgefragt und gelöscht wird und welches das ist.
sirdonat
User
Beiträge: 12
Registriert: Dienstag 10. Dezember 2013, 09:14

@BlackJack, Bitoperationen sind eingebaut...
Und zum ladeproblem der matrizen: einfach numpy.load("link/matrix.npy", mmap_mode="r")

Also noch mal vielen Dank für eure Hilfe! Habe viel gelernt.
Antworten