Parse life-like automata rules.

[Sedictious]

So I went ahead and implemented the same thing I did for Elementary Cellular Automata. The parser is admittedly a bit chunky and inelegant, so I'll add the respective docs once I'm done with the fixes.

On the plus side, the result is more interesting. Here is an automaton following the replicator rule (B1357/S1357)

[cardinot]

Hi @Sedictious, that's a good idea.

However, the standard game of life is known to be the "hello world" of ABM, and modelers usually check it to understand how the API works. So, I'd keep this model as simple as possible, i.e., only implementing the standard and more widespread scenario.

Thus, you could push this more general solution into a new repository (as a new plugin). Perhaps calling it model-gameOfLife2?. Then, I'd be happy to review the code and fork your new repo into the official Evoplex account 😉

Regarding the parser. Why do we need to pass a command (string) here?
As far as I understood, the command is always in the format Bint/Sint, right? In this case, you could just add two integer fields and let the user type the numbers directly.

[Sedictious]

@cardinot I opened a new repo here!

I decided to go with the B/S notation since it is the most widespread (and hopefully straight-forward). Even if I change the input to integers, I'm afraid there's still no way of getting rid of the parser since, contrary to Elementary Automata rules, there's no single valid range . For example: in its current form, the B112/S34 isn't valid since it contains repeating digits (however B21/S3 is accepted).

[cardinot]

@cardinot I opened a new repo here!

Excellent. I'll check it soon. 👍

I decided to go with the B/S notation since it is the most widespread (and hopefully straight-forward).

Some people use S/B, others use int/int. Anyway, the notation is ok. My concern is with the extra code and edge cases introduced by that.

Even if I change the input to integers, I'm afraid there's still no way of getting rid of the parser since, contrary to Elementary Automata rules, there's no single valid range. For example: in its current form, the B112/S34 isn't valid since it contains repeating digits (however B21/S3 is accepted).

Yes, you'd still have to check if the integer (rule) is valid. However, it would save you from extracting the integers from a string, and from checking if the string is in the right format, which is usually error-prone and may involve several extra edge cases.

Thus, you could replace

"pluginAttributesScope": [ {"rules": "string"} ],

by

"pluginAttributesScope": [
    {"birth": "int[0,max]"},
    {"survival": "int[0,max]"}
],

In this way, you can assume/trust that your LifeLike::init() function will only be triggered if only if birth and survival are both valid positive integers.

Then, in your code, it might be convenient to avoid extra loops (e.g., the function QStringList::contains used in your current solution does a linear search) by loading the input (integer) into an array of booleans, where the rules are the indexes and the existing numbers (0-8) are tagged as true.

[Sedictious]

Some people use S/B, others use int/int. Anyway, the notation is ok. My concern is with the extra code and edge cases introduced by that.

@cardinot I'm not opposed to the idea, but there is one particular edge case that is actually harder to address. Take for example the following: B1/S (note that it is different than B1/S0) which is almost impossible to handle using ints since they are non-empty by definition.

Then, in your code, it might be convenient to avoid extra loops (e.g., the function QStringList::contains used in your current solution does a linear search) by loading the input (integer) into an array of booleans, where the rules are the indexes and the existing numbers (0-8) are tagged as true.

Oh yes, I suspected that my previous solution was sub-optimal so I used a single bitstream to represent the rulestring and while I didn't time the difference, I can see a significant speed boost. I was also wondering: My current implementation silently ignores any occurrence of 9, which is an impossible number of neighbours for a valid lattice grid. Would it be better to simply throw an error?

[cardinot]

@cardinot I'm not opposed to the idea, but there is one particular edge case that is actually harder to address. Take for example the following: B1/S (note that it is different than B1/S0) which is almost impossible to handle using ints since they are non-empty by definition.

As this is the only edge case, you could use -1 to denote the case in which nobody survives.

My main concern is with the formatted string. Considering that S and B are two different and well-defined inputs, I don't see any advantage in using the Golly's notation here. Having one field for each input simplifies the code and allows you to make the model "notation free".

That being said, I think there are two potential solutions:
1)

"pluginAttributesScope": [
    {"birth": "int[0,max]"},
    {"survival": "int[-1,max]"}
],

or
2)

"pluginAttributesScope": [
    {"birth": "non-empty-string"},
    {"survival": "string"}
],

In the second case, you would just have to extract the integer from the string.

Despite the edge case for an "empty survival". I would go for the first solution because it lets the GUI aware that just numbers should be accepted (QSpinBox), which has two advantages: 1) simplifies the code as you can assume that the model will only be initialized if the inputs are valid integers; 2) explicitly tells the user that the input should be an integer (QSpinBox). Then, the model would be responsible for the data validation only (instead of type+data validation). However, it's up to you; both solutions should be fine 😄

[Sedictious]

@cardinot I ended up going with the former, since it allowed me to get rid of most ugly check conditionals. Do you think that it's worth it to check whether the sequence is monotonically increasing (e.g. 312 vs 123)? Imo both should be accepted, since the input represents a set rather than a value, but I still wonder whether more checks are needed.

[cardinot]

Do you think that it's worth it to check whether the sequence is monotonically increasing (e.g. 312 vs 123)? Imo both should be accepted, since the input represents a set rather than a value

I agree with you. both 123 and 312 represent the same thing, then both should be accepted as valid inputs. 😉

I had a very quick look at your code. A few comments:

• it seems that you won't need the raw inputs (survival and birth) elsewhere in the code, so you should consider getting rid of the m_survival and m_birth members.
• the idea of holding all the rules in a single array is good. However, it would be much better to have one array for each input. Both solutions are equivalent in terms of memory consumption, but having two arrays allows you to get rid of the + 0x0A operations, which would make your code more readable.
• currently, you get the inputs as integers in the init() function, then you convert them to string and pass them through the parseCmd function. Then, in the parseCmd function you convert the strings back to integer again. Note that you don't need to convert them to strings at all; you could read the digits using the percent operator, e.g.,

int input = 123456;
do { // we need a do-while to handle the case in which input=0
    int digit = input % 10;
    // do something with `digit`
    input /= 10;
} while(input);

• the procedures to parse birth and survival are exactly the same. So, you should have a single const function to parse one input and return the array, e.g., QBitArray ruleToArray(int rule) const. Then, in the init() you would call ruleToArray once for each input.

[Sedictious]

the idea of holding all the rules in a single array is good. However, it would be much better to have one array for each input. Both solutions are equivalent in terms of memory consumption, but having two arrays allows you to get rid of the + 0x0A operations, which would make your code more readable.

You're right, and contiguous allocation (probably) won't have any affect on performance.

currently, you get the inputs as integers in the init() function, then you convert them to string and pass them through the parseCmd function. Then, in the parseCmd function you convert the strings back to integer again.

I didn't mind this at first, since this is only called on initialization anyway, but I agree parseCmd is a lot cleaner this way.

[cardinot]

Thanks @Sedictious. 👍
Final comments:

1. L13 should pass just an int, which allows you to remove L15

2. as a best practice in c++; the parsecmd function is not supposed to change any member, so you should make it const, i.e., QBitArray LifeLike::parseCmd(int cmd) const L13. More info here

3. L13 I would prefer to call it ruleToBitArray(int rule) or parseRule(int rule) which are more informative than parseCmd

4. [cosmetic fix] it'd be good to make your coding style consistent with our guidelines, e.g., check good examples of conditionals and for-loops

5. L52 and L53; those integers are used only once, so there is no need to store them here. You could pass them directly into the parser, i.e.,

   m_birthLst = parseRule(attr("birth").toInt());
   m_survivalLst = parseRule(attr("survival").toInt());

6. L17 why 10? the valid digits are 0-8, right? then it should be better to keep only the 9 valid digits. Then, while reading the digits, if you find 9, you should send a warning message (e.g., "The rule '%1' is invalid. It should contain only unique integers (from 0 to 8).") and invalidate the input.

7. L63 you could remove this if-statement and add those checks directly at L68 as follows:

   return m_liveAttrId >= 0 && !m_birthLst.isNull() && !m_survivalLst.isNull();

[Sedictious]

I think I made all the necessary changes!

why 10? the valid digits are 0-8, right? then it should be better to keep only the 9 valid digits. Then, while reading the digits, if you find 9, you should send a warning message (e.g., "The rule '%1' is invalid. It should contain only unique integers (from 0 to 8).") and invalidate the input.

I briefly touched on it before:

My current implementation silently ignores any occurrence of 9, which is an impossible number of neighbours for a valid lattice grid. Would it be better to simply throw an error?

I ended up cutting the size down to 9

[cardinot]

I think I made all the necessary changes!

Looks good. 👌
Forgot to ask you to update the README file, could you do that please?

[Sedictious]

@cardinot Done!