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.

Sign up for GitHub

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

Jump to bottom

Adjusting access to utils for federated execution after fed-gen is done #112

Open
hokeun opened this issue Jul 24, 2022 · 6 comments · May be fixed by #217
Open

Adjusting access to utils for federated execution after fed-gen is done #112

hokeun opened this issue Jul 24, 2022 · 6 comments · May be fixed by #217

Comments

@hokeun
Copy link
Member

hokeun commented Jul 24, 2022

After fed-gen is done, the generated .lf files from a federated LF program will be the main reactors, which is currently generated as App class rather than FederatedApp class.
However, inside the network sender body (reaction body), this.util.sendRTITimedMessage will be called in the main reactor. This is currently not possible since App class doesn't implement sendRTIMessage or sendRTITimedMessage like this.

    protected sendRTIMessage<T extends Present>(data: T, destFederateID: number, destPortID: number) {
        throw new Error("Cannot call sendRTIMessage from an App. sendRTIMessage may be called only from a FederatedApp");
    }

    protected sendRTITimedMessage<T extends Present>(data: T, destFederateID: number, destPortID: number) {
        throw new Error("Cannot call sendRTIMessage from an App. sendRTIMessage may be called only from a FederatedApp");
    }

Possible solutions:

  1. Somehow we distinguish a reactor to be generated as FederatedApp and a reactor to be generated as App.
  2. Allow sendRTITimedMessage to be called inside App
  3. Other suggestions? (@lhstrh)
@hokeun hokeun changed the title Adjusting access to utils for federated execution Adjusting access to utils for federated execution after fed-gen is done Jul 24, 2022
@lhstrh
Copy link
Member

lhstrh commented Aug 16, 2023

@byeong-gil: I think this may be part-way addressed? I believe we do use the FederatedApp class, but there are still methods in the App class that belong in the FederatedApp class.

@byeonggiljun
Copy link
Collaborator

@byeong-gil: I think this may be part-way addressed? I believe we do use the FederatedApp class, but there are still methods in the App class that belong in the FederatedApp class.

Yes, I think so. We do use the FederatedApp class and the access of util functions like sendRTITImedMessage is handled properly.

@lhstrh
Copy link
Member

lhstrh commented Aug 16, 2023

But aren't they defined in App (vs. FederatedApp)?

@byeonggiljun
Copy link
Collaborator

byeonggiljun commented Aug 16, 2023 •
edited
Loading

Yes, they are. Should we define those things in FederatedApp?

Actually, those functions are defined in util as below and are overridden here. I couldn't imagine how we can get rid of them from the util class.

reactor-ts/src/core/reactor.ts

Lines 1916 to 1923 in bb9b0ea

public sendRTITimedMessage<T>(
data: T,
destFederateID: number,
destPortID: number,
time: TimeValue | undefined
): void {
this.app.sendRTITimedMessage(data, destFederateID, destPortID, time);
}

Additionally, the below function is able to be removed simply. It is not overridden and just sends an error.

reactor-ts/src/core/reactor.ts

Lines 2085 to 2094 in bb9b0ea

protected sendRTITimedMessage<T>(
data: T,
destFederateID: number,
destPortID: number,
time: TimeValue | undefined
): void {
throw new Error(
"Cannot call sendRTIMessage from an App. sendRTIMessage may be called only from a FederatedApp"
);
}

@lhstrh
Copy link
Member

lhstrh commented Aug 17, 2023

I don't think that UtilityFunctions has to be defined anonymously. We can define it in some other file and have a FederateUtilityFunctions extends UtilityFunctions that includes all the additional communication methods. We'll just instantiate it in FederatedApp (which, in hindsight, I think should actually just be called Federate).

@byeonggiljun
Copy link
Collaborator

I got it. I'll make a PR to address it and another PR or issue for renaming (e.g. FederatedApp -> Federate, TimedMessage -> TaggedMessage...).

@byeonggiljun byeonggiljun linked a pull request Aug 18, 2023 that will close this issue
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Draft: Adjusting access to utils for federated execution lf-lang/reactor-ts
3 participants
@hokeun @lhstrh @byeonggiljun