Developement/logic assertions

[Soroosh129]

During my latest hunt for a logic error in the distributed execution framework, I realized that some logic sanity checks might be really useful. For example, there is a function called _lf_advance_logical_time(instant_t next_time) that had the following basic logic:

_lf_advance_logical_time(instant_t next_time) {
	if (next_time > current_time) {
        // Advance time
    } else if (next_time == current_time) {
        // Advance microstep
    }
}

As it turned out, this function was being called incorrectly with a next_time value that was behind current_time, effectively moving logical time back. Moreover, there were situations where this function was being called correctly with a next_time in the future. However, this next_time had a value that was larger than the event tag that was at the head of the event queue. This is also incorrect logic but much more subtle.

In pursuit of these bugs, I realized that if _lf_advance_logical_time had the following structure, it would have been much easier to detect and chase these bugs:

void _lf_advance_logical_time(instant_t next_time) {
    // Check next_time against the head of the event queue
    event_t* next_event = (event_t*)pqueue_peek(event_q);
    ... // Code omitted for clarity
    if (current_tag.time < next_time) {
        // Advance time
    } else if (current_tag.time == next_time) {
        // Advance microstep
    } else {
        error_print_and_exit("_lf_advance_logical_time(): Attempted to move tag back in time.");
    }
}

This sanity check however, adds considerable overhead to the runtime (about 6msec to TimeLimit.lf). Therefore, I think we need a mechanism (a target property perhaps) to turn on these logic/sanity checks for development purposes but keep them out of end-user code.

[edwardalee]

Perhaps these checks could be enabled if logging is enabled? I think there is a simple #ifdef that could be used.

[Soroosh129]

But I think logging at first glance might be separate from debugging unless DEBUG is selected as the log level.
As an alternative to consider, some other big projects usually distinguish their debug compile from their release compile, generally by passing a flag. Having this feature can be useful in all steps of the pipeline. For example, the CGenerator can automatically add -g -rdynamic -Wall to the compile flags if debug mode is selected.

For the IDE, the debug mode could be enabled for Lingua Franca if the IDE itself is executed in debug mode (using Debug As). It seems that we might be able to detect the Java debug mode (used by Ecilpse as well) by looking for the -agentlib:jdwp or -Xrunjdwp arguments. The IDE itself is launched with the -agentlib:jdwp argument in debug mode.

[edwardalee]

I worry that having this enabled only when the IDE is launched in debug mode makes it rather invisible.
Another thought might to add a level to the logging levels between “warning” and “log” called, maybe “extra-checks” or “strict” or some such. If the logging level is set to “strict” or higher (“log” or “debug”), then additional checks are performed on various operations.

[cmnrd]

Maybe you could have a look at the C++ template for inspiration. There are two macros ASSERT and VALIDATE. ASSERT is for sanity checking your code base, i.e. if foo(int* bar) should never receive a nullpointer (its part of the contract) you do an ASSERT(bar != nullptr, "some error text") at the beginning of foo's code body. Whether assertions are active or not, is in turn controlled by the NDEBUG macro (this also controls the built-in assert). Using Cmake, NDEBUG is automatically set for the Debug build-type, and disabled for all Release build-types.

VALIDATE is used in reactor-cpp to perform additional sanity checks. For instance, it could check whether a reaction is allowed to write to a certain port or whether logical time can be advanced like in your example. VALIDATE is enabled by default, but can be disabled via the no-runtime-validation target property. This allows you to write and test your programs with sanity checks enabled. When you are sure it is doing the right thing, and you want to deploy the program, then you can disable validation for the extra bit of performance.

The intuition here is that ASSERT is used to verify correctness inside the runtime library (i.e. are all assumptions a function makes met), whereas VALIDATE is used to verify semantic correctness at user faced APIs (i.e. is a function used as intended).

I wouldn't couple this kind of functionality to the logging levels. I think they are really separate things.

[lhstrh]

Agreeing with @cmnrd that this probably should be independent of logging levels.