Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Surprising behavior involving a reactor that splits a multiport into individual ports #1321

Open
petervdonovan opened this issue Aug 7, 2022 · 6 comments
Assignees
Labels
bug Something isn't working c Related to C target
Milestone

Comments

@petervdonovan
Copy link
Collaborator

I'm labeling this as a question because it's possible this stems from a misunderstanding on my part of the semantics of multiports. I had difficulty getting any programs similar to these to work.

IndexIntoMultiportOutput

This program compiles successfully and executes with the C target, but its behavior is inconsistent with my expectations. I expect it to print "1 = 1" and "2 = 2", but instead, it prints "1 = 0" and "2 = 1". This is because instead of connecting the ith reaction to the ith multiport channel, it fills reactions into the multiport channels starting at channel 0. I find this kind of behavior strange and hard to reason about. The way messages are routed inside MultiportSplitter should not be affected by how things are connected outside of it.

Screenshot from 2022-08-06 18-39-01

This is the output:

---- Start execution at time Sat Aug  6 18:37:04 2022
---- plus 231711826 nanoseconds.
---- Using 1 workers.
1 = 0
2 = 1
---- Elapsed logical time (in nsec): 0
---- Elapsed physical time (in nsec): 208,243

This is the program:

target C

reactor ReactorWithMultiport(width: int(3)) {
    output[width] out: int

    reaction(startup) -> out {=
        for (int i = 0; i < self->width; i++) {
            lf_set(out[i], i);
        }
    =}
}

reactor MultiportSplitter(width: int(3)) {
    input[width] in: int

    output out0: int
    output out1: int
    output out2: int

    in -> out0, out1, out2
}

main reactor IndexIntoMultiportOutput {
    source = new ReactorWithMultiport()
    splitter = new MultiportSplitter()

    source.out -> splitter.in

    reaction(splitter.out1) {=
        printf("1 = %d\n", splitter.out1->value);
    =}
    reaction(splitter.out2) {=
        printf("2 = %d\n", splitter.out2->value);
    =}
}

IndexIntoMultiportInput

This program fails with the C++ target, I think during the assemble phase of execution, and I do not understand why.
index-into-multiport-input
This is the output of the program:

terminate called after throwing an instance of 'std::runtime_error'
  what():  unexpected case
Aborted (core dumped)

This is the program:

target Cpp

reactor ReactorWithMultiport {
    input[3] in: int

    reaction(in) {=
        for (auto i = 0; i < in.size(); i++) {
            if (in[i].is_present()) {
                std::cout << "received input at port " << i << std::endl;
            }
        }
    =}
}

reactor MultiportSplitter {
    output[3] out: int

    input in0: int
    input in1: int
    input in2: int

    in0, in1, in2 -> out
}

main reactor IndexIntoMultiportInput {
    splitter = new MultiportSplitter()
    receiver = new ReactorWithMultiport()

    splitter.out -> receiver.in

    reaction(startup) -> splitter.in1 {=
        splitter.in1.set(42);
    =}
}
@petervdonovan petervdonovan added question Further information is requested c Related to C target cpp Related to C++ target labels Aug 7, 2022
@cmnrd
Copy link
Collaborator

cmnrd commented Sep 7, 2022

The first example indeed looks like a bug in the C code generator. Have you tried this in the C++ target?

The error message from the C++ runtime in the second example is not very informative, but it is saying that directly connecting inputs to outputs is not implemented (as I did not consider this a valid use at the point of writing the code). As it looks, we do support direct connections in LF, so we should update the C++ runtime. In the meantime, you can work around this by adding a reaction in the middle that dimply forwards all values from the inputs to the correct output port.

@petervdonovan
Copy link
Collaborator Author

The first example indeed looks like a bug in the C code generator. Have you tried this in the C++ target?

I tried it in C++ just now and saw the same error as in the other example:

terminate called after throwing an instance of 'std::runtime_error'
  what():  unexpected case
Aborted (core dumped)

Incidentally, I was surprised that I needed to call .get() twice, although I suppose it makes sense since you have to get the port and then the value of the port. Here is the C++ version of the program:

target Cpp

reactor ReactorWithMultiport(width: int(3)) {
    output[width] out: int

    reaction(startup) -> out {=
        for (int i = 0; i < width; i++) {
            out[i].set(i);
        }
    =}
}

reactor MultiportSplitter(width: int(3)) {
    input[width] in: int

    output out0: int
    output out1: int
    output out2: int

    in -> out0, out1, out2
}

main reactor {
    source = new ReactorWithMultiport()
    splitter = new MultiportSplitter()

    source.out -> splitter.in

    reaction(splitter.out1) {=
        std::cout << "1 = " << splitter.out1.get().get() << std::endl;
    =}
    reaction(splitter.out2) {=
        std::cout << "2 = " << splitter.out2.get().get() << std::endl;
    =}
}

@cmnrd
Copy link
Collaborator

cmnrd commented Sep 8, 2022

Oh, right this is the same problem as in the other example.

In the C++ target, get() on a port always returns a smart pointer to the value. You can retrieve the actual value again with get() as you did or you can use the C++ dereference operator * (this is what I usually use).

cmnrd added a commit that referenced this issue Sep 8, 2022
This pulls in lf-lang/reactor-cpp#29 and addresses the
problem in the C++ target described in
#1321. It also adds the two
programs written by @petervdonovan as tests.
@cmnrd
Copy link
Collaborator

cmnrd commented Sep 8, 2022

I fixed the problem in the C++ runtime in #1361. Now the first program works as expected in C++.

@petervdonovan petervdonovan added bug Something isn't working and removed question Further information is requested labels Sep 8, 2022
@lhstrh
Copy link
Member

lhstrh commented Sep 28, 2022

Looks like this still has to be fixed for C?

@petervdonovan
Copy link
Collaborator Author

Yes, even after #1370 this issue seems to persist.

@cmnrd cmnrd removed the cpp Related to C++ target label Dec 2, 2022
@lhstrh lhstrh added this to the 0.9.0 milestone Jul 1, 2024
@lhstrh lhstrh modified the milestones: 0.9.0, 0.10.0 Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working c Related to C target
Projects
None yet
Development

No branches or pull requests

4 participants