New handling of startup/shutdown/reset reactions in modes #1169

a-sr · 2022-05-13T12:58:11Z

Requires reactor-c PR #79
Requires benchmarks PR #15
Requires benchmarks PR #18

Implies website PR #67
Implies website PR #68

Closes #999
Closes #1048

Implements the following semantics:

Startup reactions:
- Only triggered upon first entry into a mode
- Implication: Some legacy reactors (without reset reactions) might stop working when entered via reset because an "initial token" is not recreated upon reset entry
Shutdown reactions:
- Triggered at shutdown time of reactor
- Only for modes that were active anytime before
- Executed even is mode is inactive
New reset reactions:
- Triggered at activation time of a mode if entered via reset
New reset transition:
- No longer resets state variables
  - Can be handled by reset trigger
  - Implication: Always requires manual reset. Legacy reactors (without reset reaction) keep state variables values upon reset, leading to previous "history" leaking into the current (otherwise reset) behavior (e.g. a counter)

TODO:

Fix Python support
Fix newly blocked keywords in tested models (New Keywords for Modal Reactors #947)
Add a warning for reactor instantiations with state variables but without reset reactions

Only in C

a-sr · 2022-05-19T13:25:31Z

The implementation is complete and this PR and the related ones are ready for review.

The benchmark tests still fail because the fix in the benchmark repository is not yet merged.
The error in cli-tests on Mac looks weird but seems unrelated to this PR, since other tests succeed. @lhstrh Could you help me out with that one?

edwardalee

LGTM! The website documentation will also need to be updated.

lhstrh · 2022-05-27T05:17:20Z

org.lflang/src/org/lflang/LinguaFranca.xtext

@@ -134,7 +134,7 @@ TargetDecl:
        ((parens+='(' (init+=Expression (','  init+=Expression)*)? parens+=')')
            | (braces+='{' (init+=Expression (','  init+=Expression)*)? braces+='}')
        )?
-    ) ';'?
+    ) (reset?='reset')? ';'?


I'm confused about this syntax. What does it mean if a state variable of type foo* is marked as reset?

I also find it a bit strange to see this modifier at the end of the declaration. I think it would make more sense to have it at the beginning.

Also, I'm thinking that a keyword like transient might be better, because it is a clear adjective and a much less frequently used term in variable names.

Isn't reset already a keyword (for reset transitions)?

I always like to be able to effortlessly use programming language syntax in natural language in ways that makes sense. To me, a state ... reset is awkward, whereas transient state (or something along those lines) makes sense.

I see why that's desirable. I still think it's better to have reset state ... than state ... reset.

Do you know what the reset behavior is for state variables with a pointer types?

I added the reset as a shorthand notation for resetting the state variable in a reset reaction, so I agree with @edwardalee that it should have a connection to it.

In my mind, I do not consider the reset as an integral component of the state variable but rather a handler added to it, like a deadline or an after delay. I also did not want to draw to much attention to it by putting it first but if you think a reset state makes more sense, I can change that.

Do you know what the reset behavior is for state variables with a pointer types?

Well, the variable is reset to the initial value provided by the user. I assumed that there always has to be one but if that was a misconception, I will add an error message in case reset is used without an initial value.

Handlers go at the end, sure, but this isn't really a handler but a modifier that labels the state as being automatically reset. I suggest we put the reset in front of the state variable declaration.

lhstrh

This looks good to me, except for the syntax (as pointed out in conversation). If also really like your suggestion to validate that any state variable that is marked as reset indeed has an initial state assigned to it.

org.lflang.ui/src/org/lflang/ui/highlighting/LFSemanticHighlightingCalculator.java

initial value

declaration. Also adjusted test models to new grammar.

a-sr · 2022-05-28T09:02:18Z

Ok

added the validation error for reset state variables without initial values,
moved the reset modifier in front of the state keyword, and
adjusted the syntax examples on the website (PR68)

a-sr · 2022-05-29T06:42:46Z

Rust benchmark tests currently fail because there are newly added models using reset as variable name. Benchmarks PR #18 fixes these models.

edwardalee · 2022-06-03T05:13:03Z

Is this ready to merge?

lhstrh

These changes are looking great! My only comment is that the new validator checks should have unit tests. Other than that, this looks ready to merge.

org.lflang/src/org/lflang/validation/LFValidator.java

test/C/src/modal_models/ModalStateResetAuto.lf

Also fixed bugs in tested rules.

Also corrected display of mode-local state variables.

The basic toText functions include code correspondence tags that are irrelevant and unappealing to the diagram.

lhstrh · 2022-06-10T17:29:50Z

test/C/src/modal_models/ModalStateResetAuto.lf

+    =}
+
+    initial mode One {
+        state counter1:int(0);


It looks like the state variable isn't rendered quite correctly in the diagram:

lhstrh · 2022-06-10T17:36:34Z

org.lflang/src/org/lflang/validation/LFValidator.java

                    }
                    names.add(instantiation.getName());
                }
            }
        }
    }

+    @Check(CheckType.FAST)
+    public void checkMissingStateResetInMode(Reactor reactor) {


It appears that a reset transition is the default, meaning that is no modifier is given in the transition effect, this is to mean that it is a reset transition. This could be confusing, because it may suggest that a "normal" transition is of a different kind than reset or continue. We had a similar problem with actions, and decided that it would be better to require a disambiguation from the programmer and thus only allow logical action or physical action, and not just action. I think we should take the same approach here, and enforce this in the validator so we can present a useful error message.

lhstrh · 2022-06-10T17:37:51Z

org.lflang/src/org/lflang/LinguaFranca.xtext

+    STARTUP='startup' | SHUTDOWN='shutdown' | RESET='reset';
+
+enum ModeTransition:
+    RESET='reset' | HISTORY='continue';


I'm still a bit on the fence about the continue keyword. If refer to this a "history transition" and label it with an "H" in the diagram, then why don't we just use the history keyword and avoid all confusion?

lhstrh · 2022-06-10T17:50:04Z

test/Python/src/modal_models/ModalStateReset.lf

@@ -29,6 +29,10 @@ reactor Modal {
        state counter1(0)
        timer T1(0msec, 250msec)

+        reaction(reset) {=


The rendering of the reset trigger seems a bit off:

I would actually like to propose an alternative visual syntax. How about something along these lines:

lhstrh · 2022-06-10T17:50:57Z

org.lflang.tests/src/org/lflang/tests/compiler/LinguaFrancaValidationTest.java

@@ -2221,7 +2222,153 @@ public void testUnrecognizedTarget() throws Exception {
            "Unrecognized target: Pjthon");
    }

-
+    @Test
+    public void testInitialMode() throws Exception {


Great! Thanks for putting these in!

lhstrh

This looks really good. I raised a couple points about syntax, validation, and visual representation. If you have the bandwidth, I would prefer we address these issues as part of this PR... I'm also fine with merging and adding fixes later.

lhstrh · 2022-06-10T21:01:27Z

I'll go ahead and merge this in to avoid stalling things any further. I've created issues to track my comments.

modes: Implemented new handling of startup/shutdown/reset in modes

be1de0f

Only in C

a-sr mentioned this pull request May 13, 2022

New handling of startup/shutdown/reset reactions in modes lf-lang/reactor-c#79

Merged

a-sr added 5 commits May 13, 2022 15:14

Merge remote-tracking branch 'origin/master' into modal-models

a78d6ae

modes: Adjusted Kotlin files to new trigger API

1c7ebbc

modes: Added support for enlisting state variables for automatic reset

341a8c4

modes: Fixed state variable reset in python target

85ee331

Merge remote-tracking branch 'origin/master' into modal-models

0a728a7

a-sr mentioned this pull request May 19, 2022

Fix models that use names now blocked by modal model keywords lf-lang/benchmarks-lingua-franca#15

Merged

a-sr added c Related to C target python Related to the Python target language Related to the syntax and semantics of LF modal models labels May 19, 2022

a-sr requested a review from edwardalee May 19, 2022 13:23

a-sr marked this pull request as ready for review May 19, 2022 13:23

edwardalee approved these changes May 23, 2022

View reviewed changes

a-sr added 3 commits May 25, 2022 10:32

Merge remote-tracking branch 'origin/master' into modal-models

257dee3

Fixed merge error

0318863

Fixed missing import

bad2201

a-sr mentioned this pull request May 25, 2022

Update documentation of reset behavior of state variables in modes lf-lang/lf-lang.github.io#67

Merged

a-sr added 2 commits May 25, 2022 16:55

Merge remote-tracking branch 'origin/master' into modal-models

d3c57eb

Updated reactor-c

335c752

lhstrh reviewed May 27, 2022

View reviewed changes

lhstrh requested changes May 27, 2022

View reviewed changes

org.lflang.ui/src/org/lflang/ui/highlighting/LFSemanticHighlightingCalculator.java Show resolved Hide resolved

a-sr added 3 commits May 28, 2022 09:50

Merge remote-tracking branch 'origin/master' into modal-models

25539dd

modes: Added validation rule to check for reset state variables without

c75271c

initial value

modes: Moved reset modifier in front of state keyword in state variable

1666681

declaration. Also adjusted test models to new grammar.

a-sr mentioned this pull request May 28, 2022

Adjust syntax for resetting state variables to latest grammar changes lf-lang/lf-lang.github.io#68

Merged

modes: Fixed state reset value validation rule

330528c

a-sr mentioned this pull request May 29, 2022

Fix models that use names now blocked by modal model keywords (addendum) lf-lang/benchmarks-lingua-franca#18

Merged

lhstrh reviewed Jun 3, 2022

View reviewed changes

org.lflang/src/org/lflang/validation/LFValidator.java Show resolved Hide resolved

test/C/src/modal_models/ModalStateResetAuto.lf Outdated Show resolved Hide resolved

a-sr added 7 commits June 3, 2022 13:13

modes: Added newline in test model

1bfeac5

modes: Added unit tests for mode related validation rules.

bb9bf33

Also fixed bugs in tested rules.

diagrams: Added graphical representation of reset triggers

fb3c134

diagrams: Added graphical representation for reset state variables.

065baa8

Also corrected display of mode-local state variables.

diagrams: Removed code correspondence tags from state variable types.

4aa9170

The basic toText functions include code correspondence tags that are irrelevant and unappealing to the diagram.

Merge remote-tracking branch 'origin/master' into modal-models

fc89ee7

diagrams: Adjusted symbol spacing for state variables

5b2fd41

lhstrh reviewed Jun 10, 2022

View reviewed changes

lhstrh approved these changes Jun 10, 2022

View reviewed changes

This was referenced Jun 10, 2022

Minor issues with diagrams of modal reactors #1219

Closed

Minor issues with the syntax of modal reactors #1220

Closed

Merge branch 'master' into modal-models

51fc447

lhstrh merged commit 85fe66c into master Jun 10, 2022

lhstrh deleted the modal-models branch June 10, 2022 21:11

lhstrh added the enhancement Enhancement of existing feature label Jul 7, 2022

lhstrh changed the title ~~Implement new handling of startup/shutdown/reset reactions in modes~~ New handling of startup/shutdown/reset reactions in modes Jul 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New handling of startup/shutdown/reset reactions in modes #1169

New handling of startup/shutdown/reset reactions in modes #1169

a-sr commented May 13, 2022 •

edited

Loading

a-sr commented May 19, 2022

edwardalee left a comment

lhstrh May 27, 2022

lhstrh May 27, 2022

lhstrh May 27, 2022

edwardalee May 27, 2022

lhstrh May 27, 2022

lhstrh May 27, 2022

a-sr May 27, 2022

a-sr May 27, 2022

lhstrh May 27, 2022

a-sr May 28, 2022

lhstrh left a comment

a-sr commented May 28, 2022

a-sr commented May 29, 2022

edwardalee commented Jun 3, 2022

lhstrh left a comment

lhstrh Jun 10, 2022

lhstrh Jun 10, 2022

lhstrh Jun 10, 2022

lhstrh Jun 10, 2022

lhstrh Jun 10, 2022

lhstrh left a comment

lhstrh commented Jun 10, 2022

New handling of startup/shutdown/reset reactions in modes #1169

New handling of startup/shutdown/reset reactions in modes #1169

Conversation

a-sr commented May 13, 2022 • edited Loading

a-sr commented May 19, 2022

edwardalee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lhstrh left a comment

Choose a reason for hiding this comment

a-sr commented May 28, 2022

a-sr commented May 29, 2022

edwardalee commented Jun 3, 2022

lhstrh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lhstrh left a comment

Choose a reason for hiding this comment

lhstrh commented Jun 10, 2022

a-sr commented May 13, 2022 •

edited

Loading