Fix cycle checking when calculating min delay from nearest physical action

[lsk567]

As the title suggests, currently, when actions for a deep loop, the recursion never terminates and leads to stack overflow. This PR addresses the issue by introducing a visited set.

[edwardalee]

Thanks for catching and fixing this! It would be nice to add a test case.

[lsk567]

Is there a cleaner way to look at an input port (PortInstance) and get a connection delay from an upstream connection into the input port? @edwardalee @erlingrj

[edwardalee]

Wow that code is ugly. It's also not quite correct. E.g., suppose you have hierarchy:

   p1 ----> p2 ---/D/---> p3

this will return TimeValue.ZERO, I think, ignoring D.
Are you trying to get the minimum tag offset along the path from the eventual source to the port?
This is pretty tricky. Something like this was done by @byeonggiljun in CExtensionUtils.java, but there is a telling comment there:

        // The minimum delay calculation needs to be made in the C code because it
        // may depend on parameter values.

Also, if the port is a multiport, there could be multiple delays to it.

[lsk567]

Are you trying to get the minimum tag offset along the path from the eventual source to the port?

Yes, that is precisely what findNearestPhysicalActionTrigger was designed to do, but it wasn't doing it thoroughly.

Here is the test case I designed. The current master branch would falsely report 10 sec when tracing back from out2.

The fix in this PR now correctly identifies that the minimum path delay from out2 to the physical action is 2 sec, not 10 sec:

 --> test/C/src/federated/TriggerLoop.lf:23:3
   |
22 | reactor Hard {
23 |   output out2: int
   |   ^^^^^^^^^^^^^^^^ Found a path from a physical action to output for reactor "Hard".
The amount of delay is 2 sec.
With centralized coordination, this can result in a large number of messages to the RTI.
Consider refactoring the code so that the output does not depend on the physical action,
or consider using decentralized coordination. To silence this warning, set the target
parameter coordination-options with a value like {advance-message-interval: 10 msec}

But it has to do some gymnastics since it seems hard to get the connection delay value from an input port (the in port of the left Delay, of type RuntimeRange.Port). So far I have only found a way to access the delay from SendRange, which requires tracing from the input port to the upstream output port, hence the ugliness.

Perhaps a long-term solution is to refactor RuntimeRange to make accessing connection delays easier.

Something like this was done by @byeonggiljun in CExtensionUtils.java, but there is a telling comment there

If the parameters are not passed in on the command line, which should not be possible now, I think it should be okay?

Also, if the port is a multiport, there could be multiple delays to it.

That's true. I will augment the test case to check for this.