Skip to content

[Bug] Replace send_pickled in CARLA drivers#233

Open
mageofboy wants to merge 5 commits into
erdos-project:masterfrom
mageofboy:pseudo-async-bug
Open

[Bug] Replace send_pickled in CARLA drivers#233
mageofboy wants to merge 5 commits into
erdos-project:masterfrom
mageofboy:pseudo-async-bug

Conversation

@mageofboy

Copy link
Copy Markdown
Contributor

send_pickle is deprecated in erdos. These changes replace the functionality of send_pickle by unpickling the message then using the standard send method. This is necessary for when running the pseudo-asynchronous simulator mode.

@pschafhalter pschafhalter changed the title fix send_pickle [Bug] Replace send_pickled in CARLA drivers Mar 17, 2022
@pschafhalter

Copy link
Copy Markdown
Member

Good catch, and thanks for the PR!

It looks like storing pickled messages in these operators is redundant -- we can just stores messages directly. Could you update the PR to make that change? Concretely:

  1. Rename _pickled_messages to _messages.
  2. Don't pickle the messages. Instead, do self._messages[msg.timestamp] = msg

@ICGog

ICGog commented Mar 18, 2022

Copy link
Copy Markdown
Contributor

We we're doing this in order to move the pickling of large sensor messages from the critical the critical path. If we stop pre-pickling messages, and sending them using the send_pickle method then the long pickling time will be accounted in the end-to-end response time, affecting experiments.

@pschafhalter

Copy link
Copy Markdown
Member

Hmmm, maybe we need to re-introduce this function in ERDOS.

I was under the impression that pickling a message took a negligible amount of time, but serializing a (1920, 1080, 3) numpy array takes around 15 ms on my laptop.

@ICGog

ICGog commented Mar 20, 2022

Copy link
Copy Markdown
Contributor

Yes, I think it would be good to re-introduce this function for now in order to ensure that the driver operators do not affect our experiments.

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