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

New structural generator [WIP] #1345

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

ZinovyevaAnna
Copy link

@ZinovyevaAnna ZinovyevaAnna commented Jun 5, 2021

My concerns:
-- using string Ids in syntheticIfGenerator condition
-- new class StructurizerNode::CondtionTree itself
-- using ReadableLabelManager for more than one name type
-- DeepFirstSearcher used to visit some nodes more than once?!

@iakov iakov changed the title New structural generator New structural generator [WIP] Jun 7, 2021
@iakov
Copy link
Member

iakov commented Jun 7, 2021

Это лучше не мёрджить, чтобы история в коммитах была почище. Но обсуждать можно.

@ZinovyevaAnna
Copy link
Author

ZinovyevaAnna commented Jun 16, 2021

Мне кажется стоит обсудить способ "факторизации".
На данный момент те блоки, которые нельзя структурировать, оборачиваются условным выражением из значений влияющих на этот блок if/switch.
Но, вообще, можно ещё делать по-другому.
Так оно сейчас.

while (true) {
	__temp_1 = brick.sensor(A1).read() > 30;
	if (__temp_1) {
		__temp_2 = brick.sensor(A2).read() > 30;
		if (!__temp_2) {
			vpered();
		}
	} else {
		__temp_3 = brick.sensor(A2).read() > 30;
		if (!__temp_3) {
			povernutsya();
		}
	}
	if (__temp_1 && __temp_2 || !__temp_1 && __temp_3) {
		vlevo();
	}
	script.wait(1000);
}

А может быть так.

while (true) {
	if (brick.sensor(A1).read() > 30) {
		if (brick.sensor(A2).read() > 30) {
			selector = 1
		} else {
			vpered();
		}
	} else {
		if (brick.sensor(A2).read() > 30) {
			selector = 1
		} else {
			povernutsya();
		}
	}
	if (selector == 1) {
		vlevo();
	}
	script.wait(1000);
}

Условия не превращаются в страшные temp и читаются легче. Но сама переменная selector вроде неочевидна (значений, конечно, может быть больше одного).

Поэтому можно преобразовать как-то так.

while (true) {
	if (brick.sensor(A1).read() > 30) {
		if (brick.sensor(A2).read() > 30) {
			__next_is_subprogram_1 = true
		} else {
			vpered();
		}
	} else {
		if (brick.sensor(A2).read() > 30) {
			__next_is_subprogram_1 = true
		} else {
			povernutsya();
		}
	}
	if (__next_is_subprogram_1) {
		vlevo();
	}
	script.wait(1000);
}

Или пойти ещё дальше и придумать хороших способ генерить имена по содержанию. Тут, хотя бы, __next_is_vlevo_1;

@IKhonakhbeeva
Copy link
Contributor

Второй вариант имхо сильно лучше первого, последний конечно читается лучше, но есть чувство, что при хоть немного более сложной схеме по итогу там будет десяток __next_is subrogram_x (или текстовый аналог) и все совсем в кашу превратится из-за того, что придется писать иф-элзы и вот это вот все (тем более что есть чувство, что в таком варианте их надо будет все равно индексировать на случай глубоких деревьев, т.е. где нам надо сначала выйти на третью подпрограмму, а потом на первую). Так что вариант с селектром немного лучше (но у него тоже должна быть индексация, т.к. аналогично вышесказанному).

@ZinovyevaAnna
Copy link
Author

ZinovyevaAnna commented Jun 22, 2021

Суть варианта с селектором в том, что доп переменная всего одна и только значения это уникальные числа.
То есть в коде вполне может быть что-то типа такого

while True:
    ....
    if smth1:
        selector = 35
        break
    ....
    if smth2:
        selector = 48
        break
if selector == 35:
    ....
if selector == 48:
    ....

И вроде как в таких случаях не особо и понятно что за числа, а если делать next_is_, то становится читаемее.

@iakov
Copy link
Member

iakov commented Jun 22, 2021

Хорошая идея выбирать для продолжения конкретный блок. Но лучше это сделать отдельно. Во-первых, лучше сначала получить что-то работающее, которое уже будет радовать людей. Во-вторых, есть схожая проблема в событийной схеме, например, в Lua для Пионера. Наверное, стоит погрузиться в ту проблему тоже, чтобы пытаться решать уже более общую задачу (если вдруг это та же задача).

@iakov
Copy link
Member

iakov commented Jun 22, 2021

plugins/robots/generators/ev3/ev3RbfGenerator/ev3RbfMasterGenerator.cpp:156: error: Line is longer than 120 characters
plugins/robots/generators/trik/trikGeneratorBase/src/trikMasterGeneratorBase.cpp:35: error: Line is longer than 120 characters

@IKhonakhbeeva
Copy link
Contributor

Да, я вот сейчас наконец до конца поняла логику с селектором и почему он действительно работает
Слушай, а как тебе мысль не булевые штуки __next_is_vpered делать, а в селектор класть строку? Т.е. __selector (ну или __next_block) = "vpered", что-то такое. Это возвращает нас к if-else вместо свитча по интовым значениям, но зато это проще воспринимать в контексте "ветвление по блокам привело нас к такому-то блоку" вместо "привело ли нас ветвление к этому блоку? а к этому? а к этому?..."
Звучит вроде норм
Главное отслеживать, чтобы у разных блоков не могли оказаться одинаковые имена (для подпрограмм то легко, а вот для обычных блоков еще бы айдишку писать, видимо)

@iakov iakov force-pushed the master branch 8 times, most recently from cd7a4ab to ff6360f Compare February 11, 2023 17:45
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

Successfully merging this pull request may close these issues.

3 participants