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

[Python] add federated send class test #826

Merged
merged 12 commits into from
Jan 26, 2022

Conversation

housengw
Copy link
Contributor

@housengw housengw commented Jan 5, 2022

Currently in the Python target, when a federate sets its output to a class, the other federate somehow would try to import the sender federates’s module, causing a moduleNotFoundError.

This PR adds federated tests that sets the output port to a class to help debug the issue. Those tests are expected to fail.

This PR is a work in progress.

@lhstrh
Copy link
Member

lhstrh commented Jan 5, 2022

If it's WIP, then it's better to mark it as draft.

@housengw housengw marked this pull request as draft January 5, 2022 02:19
@housengw
Copy link
Contributor Author

housengw commented Jan 6, 2022

The tests fail locally on my machine when I do runSingleTest but somehow pass in GitHub actions.

@housengw
Copy link
Contributor Author

housengw commented Jan 14, 2022

given that the send class tests pass on the GitHub action as well as a few other machines I tried, seems like the problem is an edge case local to my machine. My machine runs on MacOS Monterey. I'll include the output running test/Python/src/federated/DistributedSendClass.lf on my machine for future reference:

Terminal running the RTI:

oysteryio-2:Python wonghouseng$ rti -i 1 -n 2
RTI: Federation ID: 1
RTI: Number of federates: 2
Starting RTI for 2 federates in federation ID 1
RTI using TCP port 15045 for federation 1.
RTI: Listening for federates.
RTI: All expected federates have connected. Starting execution.
RTI: Waiting for thread handling federate 0.
Federate 0 has resigned.
RTI: Federate 0 thread exited.
RTI: Waiting for thread handling federate 1.
Federate 1 has resigned.
RTI: Federate 1 thread exited.
RTI is exiting.

Terminal running federate a, which receives a class on an input port from federate b:

oysteryio-2:Python wonghouseng$ python3 src-gen/federated/DistributedSendClass/a/DistributedSendClass_a.py -i 1
Federation ID for executable src-gen/federated/DistributedSendClass/a/DistributedSendClass_a.py: 1
Federate 0: Connected to RTI at localhost:15045.
---- Start execution at time Fri Jan 14 12:13:41 2022
---- plus 341069000 nanoseconds.
Federate 0: Starting timestamp is: 1642191240179447000.
ModuleNotFoundError: No module named 'DistributedSendClass_b'
Federate 0: FATAL ERROR: Could not deserialize deserialized_message.
Federate 0: Connection to the RTI closed with an EOF.
---- Elapsed logical time (in nsec): 0
---- Elapsed physical time (in nsec): 9,731,000

Terminal running federate b, which sends a class on an output port to federate a:

oysteryio-2:Python wonghouseng$ python3 src-gen/federated/DistributedSendClass/b/DistributedSendClass_b.py -i 1
Federation ID for executable src-gen/federated/DistributedSendClass/b/DistributedSendClass_b.py: 1
Federate 1: Connected to RTI at localhost:15045.
---- Start execution at time Fri Jan 14 12:13:59 2022
---- plus 179424000 nanoseconds.
Federate 1: Starting timestamp is: 1642191240179447000.
^CFederate 1: Connection to the RTI closed with an EOF.
---- Elapsed logical time (in nsec): 0
---- Elapsed physical time (in nsec): 6,438,575,000

@housengw
Copy link
Contributor Author

Ok, the beefy desktop started to get the same ModuleNotFound errors as my macbook air when I ran the send class tests yesterday. I'm guessing it has to do with a python-related package that I installed? I can't pin-point which one though

@Soroosh129 Soroosh129 changed the base branch from master to python-scalability-banks January 25, 2022 17:50
@Soroosh129 Soroosh129 marked this pull request as ready for review January 26, 2022 02:08
@Soroosh129
Copy link
Contributor

Thanks for creating these tests.

As it turns out, there is a solution to this issue that only requires a minor change. To oversimplify, previously, user-written code was being loaded by treating the .py file (that contains the user-code) as an external module. However, the user code can instead be loaded from the __main__ module (which is the current top-level module that is already loaded). With this change, if the user-written class is defined or imported in the top-level preamble, it would be serializable as well.

Classes defined or imported within federate preambles, however, will not be serializable. I think this should be an expected behavior by people familiar with Python though.

Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK to me, but I'm a little out of my depth here. Can you add newlines to the ends of the files and modify RELEASES.md with a summary of the bug that it fixes?

@Soroosh129 Soroosh129 merged commit e3914ad into python-scalability-banks Jan 26, 2022
@Soroosh129 Soroosh129 deleted the python-send-class-tests branch January 26, 2022 05:07
@Soroosh129 Soroosh129 restored the python-send-class-tests branch January 26, 2022 05:07
@Soroosh129 Soroosh129 deleted the python-send-class-tests branch January 26, 2022 07:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants