From 8635b98de820f05c1318552a37fdc665621609ba Mon Sep 17 00:00:00 2001 From: Shaokai Jerry Lin Date: Mon, 20 Jan 2025 16:42:05 -0800 Subject: [PATCH 1/7] Fix cycle checking when calculating min delay from nearest physical action --- .../federated/generator/FederateInstance.java | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/lflang/federated/generator/FederateInstance.java b/core/src/main/java/org/lflang/federated/generator/FederateInstance.java index 4ce02d4793..520ae86ae7 100644 --- a/core/src/main/java/org/lflang/federated/generator/FederateInstance.java +++ b/core/src/main/java/org/lflang/federated/generator/FederateInstance.java @@ -686,22 +686,44 @@ public String toString() { * otherwise */ public TimeValue findNearestPhysicalActionTrigger(ReactionInstance reaction) { + Set visited = new HashSet<>(); + return findNearestPhysicalActionTriggerRecursive(visited, reaction); + } + + /** + * Find the nearest (shortest) path to a physical action　trigger from + * this 'reaction' in terms of minimum delay by recursively visiting + * upstream triggers and reactions until a physical action is reached. + * + * @param visited A set of reactions that have been visited used to avoid deep loops + * @param reaction The reaction to start with + * @return The minimum delay found to the nearest physical action and TimeValue.MAX_VALUE + * otherwise + */ + private TimeValue findNearestPhysicalActionTriggerRecursive(Set visited, ReactionInstance reaction) { + System.out.println(reaction); TimeValue minDelay = TimeValue.MAX_VALUE; for (TriggerInstance trigger : reaction.triggers) { if (trigger.getDefinition() instanceof Action action) { ActionInstance actionInstance = (ActionInstance) trigger; if (action.getOrigin() == ActionOrigin.PHYSICAL) { + System.out.println("Physical action: " + action); if (actionInstance.getMinDelay().isEarlierThan(minDelay)) { minDelay = actionInstance.getMinDelay(); } - } else if (action.getOrigin() == ActionOrigin.LOGICAL) { + } + else if (action.getOrigin() == ActionOrigin.LOGICAL) { + System.out.println("Logical action: " + action); + System.out.println("Depends on: " + actionInstance.getDependsOnReactions()); // Logical action // Follow it upstream inside the reactor for (ReactionInstance uReaction : actionInstance.getDependsOnReactions()) { - // Avoid a loop - if (!Objects.equal(uReaction, reaction)) { + // Avoid a potentially deep loop by checking the visited set. + if (!visited.contains(uReaction)) { + visited.add(uReaction); // Mark the upstream reaction as visited. + System.out.println("Upstream reaction: " + uReaction); TimeValue uMinDelay = - actionInstance.getMinDelay().add(findNearestPhysicalActionTrigger(uReaction)); + actionInstance.getMinDelay().add(findNearestPhysicalActionTriggerRecursive(visited, uReaction)); if (uMinDelay.isEarlierThan(minDelay)) { minDelay = uMinDelay; } @@ -713,7 +735,8 @@ public TimeValue findNearestPhysicalActionTrigger(ReactionInstance reaction) { // Outputs of contained reactions PortInstance outputInstance = (PortInstance) trigger; for (ReactionInstance uReaction : outputInstance.getDependsOnReactions()) { - TimeValue uMinDelay = findNearestPhysicalActionTrigger(uReaction); + visited.add(uReaction); // Mark the upstream reaction as visited. + TimeValue uMinDelay = findNearestPhysicalActionTriggerRecursive(visited, uReaction); if (uMinDelay.isEarlierThan(minDelay)) { minDelay = uMinDelay; } From 5f8973ae2243ca54f52dd3ad5d4899cb0f846026 Mon Sep 17 00:00:00 2001 From: Shaokai Jerry Lin Date: Thu, 23 Jan 2025 18:52:13 -0800 Subject: [PATCH 2/7] Keep tracing backward from input ports. Add test case. --- .../federated/generator/FederateInstance.java | 94 +++++++++++++------ test/C/src/federated/TriggerLoop.lf | 48 ++++++++++ 2 files changed, 114 insertions(+), 28 deletions(-) create mode 100644 test/C/src/federated/TriggerLoop.lf diff --git a/core/src/main/java/org/lflang/federated/generator/FederateInstance.java b/core/src/main/java/org/lflang/federated/generator/FederateInstance.java index 520ae86ae7..b77285b068 100644 --- a/core/src/main/java/org/lflang/federated/generator/FederateInstance.java +++ b/core/src/main/java/org/lflang/federated/generator/FederateInstance.java @@ -25,7 +25,6 @@ */ package org.lflang.federated.generator; -import com.google.common.base.Objects; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -33,6 +32,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -44,6 +44,7 @@ import org.lflang.generator.PortInstance; import org.lflang.generator.ReactionInstance; import org.lflang.generator.ReactorInstance; +import org.lflang.generator.RuntimeRange; import org.lflang.generator.TriggerInstance; import org.lflang.lf.Action; import org.lflang.lf.ActionOrigin; @@ -57,6 +58,7 @@ import org.lflang.lf.Output; import org.lflang.lf.Parameter; import org.lflang.lf.ParameterReference; +import org.lflang.lf.Port; import org.lflang.lf.Reaction; import org.lflang.lf.Reactor; import org.lflang.lf.ReactorDecl; @@ -419,20 +421,20 @@ public boolean includes(Action action) { // Look in triggers for (TriggerRef trigger : convertToEmptyListIfNull(react.getTriggers())) { if (trigger instanceof VarRef triggerAsVarRef) { - if (Objects.equal(triggerAsVarRef.getVariable(), action)) { + if (Objects.equals(triggerAsVarRef.getVariable(), action)) { return true; } } } // Look in sources for (VarRef source : convertToEmptyListIfNull(react.getSources())) { - if (Objects.equal(source.getVariable(), action)) { + if (Objects.equals(source.getVariable(), action)) { return true; } } // Look in effects for (VarRef effect : convertToEmptyListIfNull(react.getEffects())) { - if (Objects.equal(effect.getVariable(), action)) { + if (Objects.equals(effect.getVariable(), action)) { return true; } } @@ -494,7 +496,7 @@ public boolean includes(Timer timer) { // Look in triggers for (TriggerRef trigger : convertToEmptyListIfNull(r.getTriggers())) { if (trigger instanceof VarRef triggerAsVarRef) { - if (Objects.equal(triggerAsVarRef.getVariable(), timer)) { + if (Objects.equals(triggerAsVarRef.getVariable(), timer)) { return true; } } @@ -661,7 +663,7 @@ public LinkedHashMap findOutputsConnectedToPhysicalActions( for (PortInstance output : instance.outputs) { for (ReactionInstance reaction : output.getDependsOnReactions()) { TimeValue minDelay = findNearestPhysicalActionTrigger(reaction); - if (!Objects.equal(minDelay, TimeValue.MAX_VALUE)) { + if (!Objects.equals(minDelay, TimeValue.MAX_VALUE)) { physicalActionToOutputMinDelay.put((Output) output.getDefinition(), minDelay); } } @@ -691,54 +693,90 @@ public TimeValue findNearestPhysicalActionTrigger(ReactionInstance reaction) { } /** - * Find the nearest (shortest) path to a physical action　trigger from - * this 'reaction' in terms of minimum delay by recursively visiting - * upstream triggers and reactions until a physical action is reached. + * Find the nearest (shortest) path to a physical action　trigger from this 'reaction' in terms of + * minimum delay by recursively visiting upstream triggers and reactions until a physical action + * is reached. * * @param visited A set of reactions that have been visited used to avoid deep loops * @param reaction The reaction to start with + * @param pathMinDelay The amount of mininum delays accumulated from the output port to the + * reaction (previous argument) * @return The minimum delay found to the nearest physical action and TimeValue.MAX_VALUE * otherwise */ - private TimeValue findNearestPhysicalActionTriggerRecursive(Set visited, ReactionInstance reaction) { - System.out.println(reaction); + private TimeValue findNearestPhysicalActionTriggerRecursive( + Set visited, ReactionInstance reaction) { + visited.add(reaction); // Mark the reaction as visited. TimeValue minDelay = TimeValue.MAX_VALUE; for (TriggerInstance trigger : reaction.triggers) { if (trigger.getDefinition() instanceof Action action) { ActionInstance actionInstance = (ActionInstance) trigger; if (action.getOrigin() == ActionOrigin.PHYSICAL) { - System.out.println("Physical action: " + action); if (actionInstance.getMinDelay().isEarlierThan(minDelay)) { minDelay = actionInstance.getMinDelay(); } - } - else if (action.getOrigin() == ActionOrigin.LOGICAL) { - System.out.println("Logical action: " + action); - System.out.println("Depends on: " + actionInstance.getDependsOnReactions()); + } else if (action.getOrigin() == ActionOrigin.LOGICAL) { // Logical action // Follow it upstream inside the reactor for (ReactionInstance uReaction : actionInstance.getDependsOnReactions()) { // Avoid a potentially deep loop by checking the visited set. if (!visited.contains(uReaction)) { - visited.add(uReaction); // Mark the upstream reaction as visited. - System.out.println("Upstream reaction: " + uReaction); TimeValue uMinDelay = - actionInstance.getMinDelay().add(findNearestPhysicalActionTriggerRecursive(visited, uReaction)); + actionInstance + .getMinDelay() + .add(findNearestPhysicalActionTriggerRecursive(visited, uReaction)); if (uMinDelay.isEarlierThan(minDelay)) { minDelay = uMinDelay; } } } } - - } else if (trigger.getDefinition() instanceof Output) { - // Outputs of contained reactions - PortInstance outputInstance = (PortInstance) trigger; - for (ReactionInstance uReaction : outputInstance.getDependsOnReactions()) { - visited.add(uReaction); // Mark the upstream reaction as visited. - TimeValue uMinDelay = findNearestPhysicalActionTriggerRecursive(visited, uReaction); - if (uMinDelay.isEarlierThan(minDelay)) { - minDelay = uMinDelay; + } else if (trigger.getDefinition() instanceof Port) { + PortInstance port = (PortInstance) trigger; + // Regardless of whether the port is an output or input port of + // a contained reactor, recurse on reactions that write to it. + for (ReactionInstance uReaction : port.getDependsOnReactions()) { + // Ports can form a loop also. + if (!visited.contains(uReaction)) { + TimeValue uMinDelay = findNearestPhysicalActionTriggerRecursive(visited, uReaction); + if (uMinDelay.isEarlierThan(minDelay)) { + minDelay = uMinDelay; + } + } + } + // The trigger is an input of a contained reactor. If it + // another upstream contained reactor connects to it, trace to + // the upstream contained reactor's output ports. + if (trigger.getDefinition() instanceof Input) { + var upstreamPorts = port.eventualSources(); + for (RuntimeRange range : upstreamPorts) { + PortInstance upstreamPort = range.instance; + + // Get a connection delay value between upstream port and + // the current port. + TimeValue connectionDelay = + upstreamPort.getDependentPorts().stream() + .filter( + sRange -> + sRange.destinations.stream() + .map(it -> it.instance) + .anyMatch(port::equals)) + .map(sRange -> ASTUtils.getDelay(sRange.connection.getDelay())) + .filter(Objects::nonNull) + .map(TimeValue::fromNanoSeconds) + .findFirst() + .orElse(TimeValue.ZERO); + + for (ReactionInstance uReaction : upstreamPort.getDependsOnReactions()) { + if (!visited.contains(uReaction)) { + TimeValue uMinDelay = + connectionDelay.add( + findNearestPhysicalActionTriggerRecursive(visited, uReaction)); + if (uMinDelay.isEarlierThan(minDelay)) { + minDelay = uMinDelay; + } + } + } } } } diff --git a/test/C/src/federated/TriggerLoop.lf b/test/C/src/federated/TriggerLoop.lf new file mode 100644 index 0000000000..fa55585984 --- /dev/null +++ b/test/C/src/federated/TriggerLoop.lf @@ -0,0 +1,48 @@ +/** + * When there is an output in the federate reactor, the compiler checks if the output is + * (transitively) connected to a physical action inside the same reactor, as the physical action + * could result in generating a large number of NULL messages. As the compiler looks for the + * physical action recursively, it should be able to handle non-trivial "trigger loop" formed by a + * set of actions. This tests check if the compiler can successfully handle programs with such a + * pattern without getting stack overflow. + */ +target C + +reactor Simple { + // This output is required to trigger finding the closest upstream physical action inside the reactor. + output out: int + logical action a + logical action b + + reaction(a) -> b {= =} + + reaction(b) -> a, out {= =} +} + +reactor Hard { + output out2: int + @label( + "We want to find this physical action from out2. The compiler should warn about min path delay = 2 sec.") + physical action d(1 sec) + @label("This physical action has a larger min delay. The compiler should ignore it.") + physical action e(10 sec) + delay = new Delay() + delay2 = new Delay() + delay.out -> delay2.in after 1 sec + + reaction(d, delay2.out) -> delay.in {= =} + + reaction(e, delay2.out) -> d, out2 {= =} +} + +reactor Delay { + input in: int + output out: int + + reaction(in) -> out {= =} +} + +federated reactor { + simple = new Simple() + hard = new Hard() +} From 1ec934c627633465f76fbfce9362d92fd85d1d9a Mon Sep 17 00:00:00 2001 From: Shaokai Jerry Lin Date: Sat, 25 Jan 2025 15:45:47 -0800 Subject: [PATCH 3/7] Add a timeout to TriggerLoop.lf --- test/C/src/federated/TriggerLoop.lf | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/C/src/federated/TriggerLoop.lf b/test/C/src/federated/TriggerLoop.lf index fa55585984..ce2c7f9136 100644 --- a/test/C/src/federated/TriggerLoop.lf +++ b/test/C/src/federated/TriggerLoop.lf @@ -6,7 +6,9 @@ * set of actions. This tests check if the compiler can successfully handle programs with such a * pattern without getting stack overflow. */ -target C +target C { + timeout: 1 msec +} reactor Simple { // This output is required to trigger finding the closest upstream physical action inside the reactor. From 21d9a98fe732b73020d53eeb491297f64459923b Mon Sep 17 00:00:00 2001 From: Shaokai Lin Date: Tue, 4 Feb 2025 11:51:11 -0800 Subject: [PATCH 4/7] Add a future todo comment --- .../org/lflang/federated/generator/FederateInstance.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/lflang/federated/generator/FederateInstance.java b/core/src/main/java/org/lflang/federated/generator/FederateInstance.java index b77285b068..d0f784d5f4 100644 --- a/core/src/main/java/org/lflang/federated/generator/FederateInstance.java +++ b/core/src/main/java/org/lflang/federated/generator/FederateInstance.java @@ -753,7 +753,12 @@ private TimeValue findNearestPhysicalActionTriggerRecursive( PortInstance upstreamPort = range.instance; // Get a connection delay value between upstream port and - // the current port. + // the current port. This is currently done by finding + // the corresponding SendRange from the upstream port + // and then extracting a delay value from the connection + // contained in the SendRange. + // TODO: Find a better way to do this, which likely involves + // refactoring SendRange, RuntimeRange.Port, and PortInstance. TimeValue connectionDelay = upstreamPort.getDependentPorts().stream() .filter( From e1c2744b396c05dc9f0697312eebdf624f6f8337 Mon Sep 17 00:00:00 2001 From: Shaokai Lin Date: Thu, 6 Feb 2025 16:02:27 -0800 Subject: [PATCH 5/7] Add multiport to test case --- test/C/src/federated/TriggerLoop.lf | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/C/src/federated/TriggerLoop.lf b/test/C/src/federated/TriggerLoop.lf index ce2c7f9136..bebacebed3 100644 --- a/test/C/src/federated/TriggerLoop.lf +++ b/test/C/src/federated/TriggerLoop.lf @@ -24,10 +24,12 @@ reactor Simple { reactor Hard { output out2: int @label( - "We want to find this physical action from out2. The compiler should warn about min path delay = 2 sec.") + "We want to find THIS physical action from out2. The compiler should warn about min path delay = 2 sec.") physical action d(1 sec) @label("This physical action has a larger min delay. The compiler should ignore it.") physical action e(10 sec) + @label("This physical is also not the one that leads to a minimal path.") + physical action f(2 sec) delay = new Delay() delay2 = new Delay() delay.out -> delay2.in after 1 sec @@ -35,10 +37,12 @@ reactor Hard { reaction(d, delay2.out) -> delay.in {= =} reaction(e, delay2.out) -> d, out2 {= =} + + reaction(f) -> delay.in {= =} } reactor Delay { - input in: int + input[2] in: int output out: int reaction(in) -> out {= =} From 21f4cca638d0e8edf5c0677224d3634921aa6910 Mon Sep 17 00:00:00 2001 From: Shaokai Lin Date: Thu, 6 Feb 2025 16:47:33 -0800 Subject: [PATCH 6/7] Make sure there are no unconnected inputs --- test/C/src/federated/TriggerLoop.lf | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/C/src/federated/TriggerLoop.lf b/test/C/src/federated/TriggerLoop.lf index bebacebed3..9e808eb5a7 100644 --- a/test/C/src/federated/TriggerLoop.lf +++ b/test/C/src/federated/TriggerLoop.lf @@ -30,8 +30,8 @@ reactor Hard { physical action e(10 sec) @label("This physical is also not the one that leads to a minimal path.") physical action f(2 sec) - delay = new Delay() - delay2 = new Delay() + delay = new Delay(width = 2) + delay2 = new Delay(width = 1) delay.out -> delay2.in after 1 sec reaction(d, delay2.out) -> delay.in {= =} @@ -41,8 +41,8 @@ reactor Hard { reaction(f) -> delay.in {= =} } -reactor Delay { - input[2] in: int +reactor Delay(width: int = 1) { + input[width] in: int output out: int reaction(in) -> out {= =} From 4fc4e4fe9e404e32b63e2d4150680645459ae1b2 Mon Sep 17 00:00:00 2001 From: Shaokai Lin Date: Thu, 6 Feb 2025 21:02:57 -0800 Subject: [PATCH 7/7] Apply spotless --- test/C/src/federated/TriggerLoop.lf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/C/src/federated/TriggerLoop.lf b/test/C/src/federated/TriggerLoop.lf index 9e808eb5a7..186e8ac9de 100644 --- a/test/C/src/federated/TriggerLoop.lf +++ b/test/C/src/federated/TriggerLoop.lf @@ -30,8 +30,8 @@ reactor Hard { physical action e(10 sec) @label("This physical is also not the one that leads to a minimal path.") physical action f(2 sec) - delay = new Delay(width = 2) - delay2 = new Delay(width = 1) + delay = new Delay(width=2) + delay2 = new Delay(width=1) delay.out -> delay2.in after 1 sec reaction(d, delay2.out) -> delay.in {= =}