Kappa-Dev / KappaTools

Tool suite for kappa models. Documentation and binaries can be found in the release section. Try it online at
http://kappalanguage.org/
GNU Lesser General Public License v3.0
108 stars 40 forks source link

False error from KaSa: mod, tokens, `<-` #661

Open hmedina opened 1 year ago

hmedina commented 1 year ago
%agent: timer(s{tik, tok})      %init: 1 timer()
%token: io      %init: 0 io     %obs: 'io_o' |io|
%token: oi      %init: 0 oi     %obs: 'oi_o' |oi|

timer(s{tik/tok}) | 1 io @ 10
timer(s{tok/tik}) | 1 oi @ 10
%mod: [T] > 5 do io <- 0 ;
%mod: [T] > 10 do $STOP;

The UI static analyzer reports 1/1[model.ka] Incorrect beginning of sentence, with a red squiggly under the io in the first mod.

Ignoring this message and hitting play produces the expected model behavior.

This syntax was referred by https://github.com/Kappa-Dev/KappaTools/issues/607 The verbose version of the syntax is tested in https://github.com/Kappa-Dev/KappaTools/blob/master/tests/integration/compiler/tokens/tokens.ka

hmedina commented 1 year ago
%mod: [T] > 5 do $SNAPSHOT ; io <- 0 ;

With compound perturbations, weird things occur. The snapshot is not present; and KaSim CLI will refuse to run the model...

feret commented 1 year ago

Dear Hector, The UI has never been adapted to KaSa error messages. Most of them are badly reported without any relation with the alarm sent. I will have to sort out and filter most of them out, while trying to understand how the UI is working.

feret commented 1 year ago

%mod: [T] > 5 do io <- 0 ; is rejected by KaSim, not KaSa.

feret commented 1 year ago

This construction is not compliant with the klexer4/kparser4. I have to update them accordingly.

feret commented 1 year ago

I extended the lexer and the parser in a branch. Then, the issue is that it created a shift/reduce conflict in the grammar. I am not sure I know how to change it without changing the syntax. Adding a keyword %foo would solve the issue.

Would it be OK with you @hmedina ? If someone knows how to deal with the parser to remove the shift/reduce conflict, please feel free to contribute.

hmedina commented 1 year ago

It would be ok with my usage (assuming it's just adding some keywords to make the "update token Y to value X" statement less cryptic...)

To elaborate, since the UI rejected that syntax, none of my big models use tokens & perturbations, because I did not know what a valid syntax would be. I only stumbled into working syntax when building a comprehensive KaTIE test, so changing the syntax will only cost me the time to re-write tests here and there.

The maintainers for both Kappa manuals will need to update their respective works once the new syntax is defined.

feret commented 1 year ago

I have solved the conflict by restricting a little bit the grammar. Now token <- alg assignments are allowed if

a) the perturbation has a ending condition (repeat [ ]) as in %mod: [T] > 5 do io <- 0 repeat [true]

b) or if it is in a list of perturbations bounded by some parentheses. as in %mod: [T] > 5 do (io <- 0)

Otherwise, it is not allowed. For instance %mod: [T] > 5 do io <- 0 is not OK.

feret commented 1 year ago

Since, the suggestion is strictly more expressive than what is in the master branch I merge.

pirbo commented 1 year ago

I'm not there anymore to do the maintenance so I don't have a voice but just to say: You don't need any extra syntax to set a number of token to a given value in a %mod, you can simply $apply the rule that does that!

To remove all the io after 5 tu, you %mod: [T] > 5 do $APPLY 1 | -|io| io;

feret commented 1 year ago

@pirbo, you will keep a voice on the development of KappaTools forever and we deeply respect your legacy. Yet, I think that even if it can be encoded, a direct syntax is more understandable from a user perspectives. Also it is important that the tools and the doc are coherent with respect to each other.

This is why I would prefer to add the new notation. (Also the impact on the code is quite narrow, since it is replaced by your proposition in the yacc file).

hmedina commented 1 year ago

This raises an issue with the repeat clause being optional & defaulting to false:

%mod: [T] > 5 do io <- 0 ;                  // rejected
%mod: [T] > 5 do ( io <- 0 ) ;              // rejected
%mod: [T] > 5 do io <- 0 ; repeat [false]   // accepted

The first statement is rejected; however I was under the impression it defaulted to the third statement. Should repeat clauses be always required? This would trigger issues like https://github.com/Kappa-Dev/KappaTools/issues/643

feret commented 1 year ago

Here what is accepted : %mod: [T] > 5 do ( io <- 0 ) // rejected %mod: [T] > 5 do io <- 0 ; repeat [false] // accepted

Shall I authorize / require a semicolon after (io <- 0) ?

feret commented 1 year ago

%mod: [T] > 5 do io <- 0 ; // rejected %mod: [T] > 5 do ( io <- 0 ) ; // accepted %mod: [T] > 5 do ( io <- 0 ) // accepted %mod: [T] > 5 do io <- 0 ; repeat [false] // accepted

hmedina commented 1 year ago

How should multi-pertubation statements be constructed?

In KaSimInBrowser (today's?), the following are rejected:

%mod: [T] > 5 do (io <- 0) ; (oi <- 0) ;
%mod: [T] > 5 do (io <- 0) ; (oi <- 0) ; repeat [false]
%mod: [T] > 5 do (io <- 0) (oi <- 0) ;
%mod: [T] > 5 do (io <- 0) (oi <- 0) ; repeat [false]
feret commented 1 year ago

The way the grammar has been written, composite statements should be written as follows: %mod: [T] > 5 do (io <- 0 ; oi <- 0) ; %mod: [T] > 5 do (io <- 0 ; oi <- 0) ; repeat [false]

hmedina commented 1 year ago

Ok, so for documentation:

someone should update table 2.7 of the official Kappa manual

@plucky should update "Grammar 13: Intervention directives" in https://kappalanguage.org/sites/kappalanguage.org/files/inline-files/Kappa_Manual.pdf