Skip to content
This repository has been archived by the owner on Nov 1, 2024. It is now read-only.

LabGraph Monitor API Improvement #77

Open
jfResearchEng opened this issue May 1, 2022 · 3 comments · May be fixed by #86
Open

LabGraph Monitor API Improvement #77

jfResearchEng opened this issue May 1, 2022 · 3 comments · May be fixed by #86

Comments

@jfResearchEng
Copy link
Contributor

jfResearchEng commented May 1, 2022

🚀 Feature

LabGraph Monitor currently needs a user to define the websocket connections when generating the yaml file passed to the frontend. Example for labgraph_monitor_example.py (link):

    def connections(self) -> lg.Connections:
        return (
            (self.NOISE_GENERATOR.NOISE_GENERATOR_OUTPUT, self.ROLLING_AVERAGER.ROLLING_AVERAGER_INPUT),
            (self.NOISE_GENERATOR.NOISE_GENERATOR_OUTPUT, self.AMPLIFIER.AMPLIFIER_INPUT),
            (self.NOISE_GENERATOR.NOISE_GENERATOR_OUTPUT, self.ATTENUATOR.ATTENUATOR_INPUT),
            (self.NOISE_GENERATOR.NOISE_GENERATOR_OUTPUT, self.SERIALIZER.SERIALIZER_INPUT_1),
            (self.ROLLING_AVERAGER.ROLLING_AVERAGER_OUTPUT, self.SERIALIZER.SERIALIZER_INPUT_2),
            (self.AMPLIFIER.AMPLIFIER_OUTPUT, self.SERIALIZER.SERIALIZER_INPUT_3),
            (self.ATTENUATOR.ATTENUATOR_OUTPUT, self.SERIALIZER.SERIALIZER_INPUT_4),
            (self.SERIALIZER.SERIALIZER_OUTPUT, self.WS_SERVER_NODE.topic),
        )

Can we simplify the the API to the following instead:

    def connections(self) -> lg.Connections:
        return (
            (self.NOISE_GENERATOR.NOISE_GENERATOR_OUTPUT, self.ROLLING_AVERAGER.ROLLING_AVERAGER_INPUT),
            (self.NOISE_GENERATOR.NOISE_GENERATOR_OUTPUT, self.AMPLIFIER.AMPLIFIER_INPUT),
            (self.NOISE_GENERATOR.NOISE_GENERATOR_OUTPUT, self.ATTENUATOR.ATTENUATOR_INPUT),
        )

Additional context

  1. Existing application can be found [here] (https://github.com/facebookresearch/labgraph/tree/main/extensions/yaml_support)
  2. The code should be added at folder is https://github.com/facebookresearch/labgraph/tree/main/extensions/yaml_support
  3. Create setup.py and README.md, where example can be found at: https://github.com/facebookresearch/labgraph/tree/main/extensions/labgraph_viz
  4. Add github action support, reference: https://github.com/facebookresearch/labgraph/actions/workflows/main.yml
  5. Add proper license header, example:
#!/usr/bin/env python3
# Copyright 2004-present Facebook. All Rights Reserved.
@AlexeyAulov
Copy link

AlexeyAulov commented Jun 16, 2022

Hi There,
Just to clarify I need to simplify the WebSocket connections in the labgraph_monitor_example.py itself? Also I can simplify the WebSocket connections, but that mean I would need to take out the serializer, I understand serializer is used to store data in bytes so when we run the example we wouldn't need to have it serialized? I can't think of a way to simplify the WebSocket connection without taking out the serializer function. Maybe I can include it into the attenuator, amplifier, noise generator, and rolling average so it is converted automatically when doing the function, but that is just a theory.

@dtemir
Copy link
Contributor

dtemir commented Jul 22, 2022

Hi @AlexeyAulov!

So, I know you wrote this a while back, but I figured I'll reply in case you're still working on this.

Just to clarify I need to simplify the WebSocket connections in the labgraph_monitor_example.py itself?

WebSocket connections is totally fine as it is right now. It was built way before us.

The connections @jfResearchEng is talking about here are between different nodes in the example, like (self.NOISE_GENERATOR.NOISE_GENERATOR_OUTPUT, self.SERIALIZER.SERIALIZER_INPUT_1)

I couldn't developer a better way to do real-time messaging without using different Topics like SERIALIZER_INPUT_1 and SERIALIZER_INPUT_2

can't think of a way to simplify the WebSocket connection without taking out the serializer function.

The Serializer part is crucial as it collects data about the entire Graph, puts it into a Python Dictionary, and sends it off using WebSocket. Without the Serializer, there is no way to communicate topology to the WebSocket.

I like the last point you make:

Maybe I can include it into the attenuator, amplifier, noise generator, and rolling average so it is converted automatically when doing the function, but that is just a theory.

But the issue I'm seeing is that the real-time data needs to come together inside the Serializer because it is the only one communicating with WebSocket.

@dtemir
Copy link
Contributor

dtemir commented Jul 22, 2022

@AlexeyAulov you can always find me on our Fellowship Discord if you ever want to discuss more! Maybe we can even with @bennaaym who is an expert on everything LabGraph Monitor related

@AlexeyAulov AlexeyAulov linked a pull request Jul 25, 2022 that will close this issue
7 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants