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

Rewrite interface #28

Open
Marenz opened this issue May 8, 2024 · 10 comments
Open

Rewrite interface #28

Marenz opened this issue May 8, 2024 · 10 comments
Assignees
Labels
part:❓ We need to figure out which part is affected priority:❓ We need to figure out how soon this should be addressed type:enhancement New feature or enhancement visitble to users

Comments

@Marenz
Copy link
Contributor

Marenz commented May 8, 2024

What's needed?

See #22 (comment) for details

Proposed solution

No response

Use cases

No response

Alternatives and workarounds

No response

Additional context

No response

@Marenz Marenz added part:❓ We need to figure out which part is affected priority:❓ We need to figure out how soon this should be addressed type:enhancement New feature or enhancement visitble to users labels May 8, 2024
@Marenz Marenz self-assigned this May 8, 2024
@llucax
Copy link
Contributor

llucax commented Jul 24, 2024

Some other ideas, after a long discussion.

If we make the running_status_change channel send events mapped 1-1 with a singular dispatch object, we can add some utility functions to merge dispatches to cope with different actor's needs.


For example, for the power setting actor, since it works with component pools, it would make sense to merge dispatches with the same selector, which will map with one component pool.

In this case:

  1. dispatch_id=1, type=set_power, selector=component_id:[1], start=1pm, duration=2h, power_w=1000
  2. dispatch_id=2, type=set_power, selector=component_id:[1], start=2pm duration=2h, power_w=1000
  3. dispatch_id=3, type=set_power, selector=component_id:[1,2], start=2pm duration=1h, power_w=2000
  4. dispatch_id=4, type=set_power, selector=component_id:[1], start=3pm duration=2h, power_w=2000

We would normally receive these events:

  • 1pm: start 1 with component_id=[1] and 1000w
  • 2pm: start 2 with component_id=[1] and 1000w, start 3 with component_id=[1,2] and 2000w
  • 3pm: stop 1, stop 3 and start 4 with component_id=[1] and 2000w
  • 4pm: stop 2
  • 5pm: stop 3

But it would make more sense to receive this (dispatch_id info is lost):

  • 1pm: start with component_id=[1] and 1000w
  • 2pm: start with component_id=[1,2] and 2000w
  • 3pm: update with 2000w, stop with component_id=[1,2]
  • 5pm: stop with component_id=[1]

The code could look like this:

async for dispatch_event in merge_same_selector(dispatcher.running_status_change.new_receiver()):
    match dispatch_event:
        Start(dispatch):
            actor = PowerSettingActor(dispatch)
            actor.start()
            self._actors[dispatch.selector] = actor
        Stop(dispatch):
            if actor := self._actors.get(dispatch.selector):
                await actor.stop()
        Update(dispatch):
            if actor := self._actors.get(dispatch.selector):
                await actor.reconfigure(dispatch)

Where merge_same_selector() does the merging.


Another example, for FCR, we want to override any overlapping dispatches:

  1. dispatch_id=1, type=set_power, selector=component_id:[1], start=1pm, duration=2h, max_power_w=1000
  2. dispatch_id=2, type=set_power, selector=component_id:[1,2], start=2pm duration=2h, max_power_w=1000
  3. dispatch_id=2, type=set_power, selector=component_id:[1], start=3pm duration=2h, max_power_w=2000

We would normally receive these events:

  • 1pm: start 1 with component_id=[1] and 1000w
  • 2pm: start 2 with component_id=[1,2] and 1000w
  • 3pm: stop 1 and start 3 with component_id=[1] and 2000w
  • 4pm: stop 2
  • 5pm: stop 3

But it would make more sense to receive this (dispatch_id info is lost):

  • 1pm: start with component_id=[1] and 1000w
  • 2pm: update with component_id=[1,2] and 1000w
  • 3pm: update with component_id=[1] and 2000w
  • 5pm: stop

The code will basically look the same but applying a different filter:

async for dispatch_event in merg_overlapping(dispatcher.running_status_change.new_receiver()):
    match dispatch_event:
        ...

@llucax
Copy link
Contributor

llucax commented Jul 24, 2024

To be able to do this, maybe we need to update the Dispatch.dispatch_id to be something like int | Merged (where Merged is a sentinel class).

@Marenz
Copy link
Contributor Author

Marenz commented Sep 11, 2024

Instead of having int | Merged, the merger functions could return something like

MergedEvent:
    event: DispatchEvent # start/stop/config
    dispatches: list[Dispatch] # list of involved dispatches

after all, the event part is what one usually cares about?

@llucax
Copy link
Contributor

llucax commented Sep 11, 2024

I think you care about the dispatch contents, at least the payload. If you get a MergedEvent(Config, [dispatch1, dispatch2, dispatch3]) how do you know which dispatch to use to reconfigure the actor?

@Marenz
Copy link
Contributor Author

Marenz commented Sep 11, 2024

I suppose they could be sorted by latest update_time and there could be a

MergedEvent:
    event: DispatchEvent # start/stop/config
    dispatches: list[Dispatch] # list of involved dispatches, ordered by modification  time

    @property
    def dispatch() -> Dispatch:
        """Return the most recently modified dispatch"""
        return self.dispatches[0]

@llucax
Copy link
Contributor

llucax commented Sep 11, 2024

Why latest update time? If you have one dispatch id=2 from 10 to 11, and dispatch id=3 from 11 to 12, and it is 10:30 and someone updates dispatch id=3, then the returned dispatch should be id=2 even when the last updated dispatch is id=3, right? I think you would need to search which dispatch is currently active based on the current time, no?

@Marenz
Copy link
Contributor Author

Marenz commented Sep 11, 2024

i'd say there would be no event at all in your example because it wouldn't change the current running state? Only at 11 it would trigger an event containing only id 3?

@llucax
Copy link
Contributor

llucax commented Sep 11, 2024

Yeah, if the payload changed, a "reconfigure"/"update" event should be sent so the actor can act on it.

@llucax
Copy link
Contributor

llucax commented Sep 11, 2024

Only at 11 it would trigger an event containing only id 3?

Yeah, that's what I was thinking of. To put more context to the example, lets say disaptch id=2 says charge 5kw and id=3 says charge 1kw. The actor should switch from 5kw to 1 kw at 11, right?

@Marenz
Copy link
Contributor Author

Marenz commented Nov 4, 2024

Yeah, that's what I was thinking of. To put more context to the example, lets say disaptch id=2 says charge 5kw and id=3 says charge 1kw. The actor should switch from 5kw to 1 kw at 11, right?

That sounds about right,, also, github copilot wants to say that:

In the context of the given scenario outlined in the issue comment, the actor should switch from 5kW to 1kW at 11 if the dispatch logic prioritizes merging or overriding dispatches based on the component's needs and the specific filter being applied (e.g., merge_same_selector or merg_overlapping). This approach ensures efficient power management and adherence to the dispatch requirements.

@Marenz Marenz closed this as completed Nov 4, 2024
@Marenz Marenz reopened this Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
part:❓ We need to figure out which part is affected priority:❓ We need to figure out how soon this should be addressed type:enhancement New feature or enhancement visitble to users
Projects
None yet
Development

No branches or pull requests

2 participants