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

Consider abstracting component pool groups into their own class #963

Open
llucax opened this issue Jun 10, 2024 · 0 comments
Open

Consider abstracting component pool groups into their own class #963

llucax opened this issue Jun 10, 2024 · 0 comments
Labels
part:actor Affects an actor ot the actors utilities (decorator, etc.) part:power-management Affects the management of battery power and distribution type:tech-debt Improves the project without visible changes for users
Milestone

Comments

@llucax
Copy link
Contributor

llucax commented Jun 10, 2024

What's needed?

We have now 2 component pool groups: the default and the shifting group (which sort of move the baseline of the default group). The code could be restructured a bit to avoid some duplication.

Proposed solution

Something like:

class PoolGroup:
    algorithm: BaseAlgorithm
    subscriptions: dict[...]
    # Maybe more stuff could be stored directly in the group,
    # like the `component_ids` and `system_bounds`, so this
    # methods will need even fewer arguments, but I didn't
    # check if this is feasible or not

    def subscribe(...) -> None: ...
        if component_ids not in self.subscriptions:
            self.subscriptions[component_ids] = {
                priority: self._channel_registry.get_or_create(
                    _Report, subcription_request.get_channel_name()
                ).new_sender()
            }
        elif priority not in self.subscriptions[component_ids]:
            self.subscriptions[component_ids][priority] = (
                self._channel_registry.get_or_create(
                    _Report, subcription_request.get_channel_name()
                ).new_sender()
            )

    def calculate_target_power(
        self,
        component_ids: frozenset[int],
        system_bounds: ...,
        *,
        proposal: Proposal | None = None,
        shift: Power | None = None,
        must_send: bool = False,
     ) -> Power | None:
        bounds = self._calculate_shifted_bounds(system_bounds[component_ids], shift)
        return self.algorithm.calculate_target_power(
                    component_ids, proposal, bounds, must_send,
                )

    async def send_report(
        self, component_ids: frozenset[int], distribution_results: ..., bounds: ..., *,
        shift: Power | None = None
    ) -> None:
        for priority, sender in self.subscriptions.get(component_ids, {}).items():
            bounds = self._calculate_shifted_bounds(bounds, shift)
            result = distribution_results.get(component_ids)
            status = self.algorithm.get_status(component_ids, priority, bounds, result)
            await sender.send(status)

    def _calculate_shifted_bounds(
        self, bounds: SystemBounds, shift: Power | None
    ) -> SystemBounds:
        if shift is None:
            return bounds
        ...

class PowerManagingActor(Actor):
    def __init__(self, ...):
        self._default_group = PoolGroup(...)
        self._shifting_group = PoolGroup(...)

    async def _run(self) -> None:
        async for selected in select(...):
            if ...:
                ...
            elif selected_from(selected, self._bounds_subscription_receiver):
                sub = selected.message
                component_ids = sub.component_ids
                priority = sub.priority
                group = self._shifting_group if sub.in_shifting_group else self._default_group
                group.subscribe(...)
		        if component_ids not in self._bound_tracker_tasks:
		            self._add_system_bounds_tracker(component_ids)        

    def _calculate_target_power(self, ...) -> Power | None:
        tgt_power_shift: Power | None = None
        tgt_power_default: Power | None = None
        if proposal is not None:
            if proposal.in_shifting_group:
                bounds = self._system_bounds[component_ids]
                tgt_power_shift = self._shifting_group.calculate_target_power(
                    component_ids, bounds, proposal=proposal, must_send=must_send,
                )
                tgt_power_no_shift = self._default_group.calculate_target_power(
                    component_ids, bounds, shift=tgt_power_shift, must_send=must_send,
                )
        ...

    async def _send_reports(self, component_ids: frozenset[int]) -> None:
        if bounds is None:
            _logger.warning("PowerManagingActor: No bounds for %s", component_ids)
            return
        self._shifting_group.send_reports(component_ids, self._distribution_results, bounds)
        self._default_group.send_reports(
            component_ids,
            self._distribution_results,
            bounds,
            shift=self._shifting_group.algorithm.get_target_power(component_ids),
        )

Additional context

The original suggestion comes from the PR introducing the groups:

@llucax llucax added type:tech-debt Improves the project without visible changes for users part:power-management Affects the management of battery power and distribution part:actor Affects an actor ot the actors utilities (decorator, etc.) labels Jun 10, 2024
@llucax llucax added this to the Untriaged milestone Jun 10, 2024
@llucax llucax modified the milestones: Untriaged, v1.0.0, v1.0.0-rc800 Jul 29, 2024
@shsms shsms modified the milestones: v1.0.0-rc800, v1.0.0-rc900 Aug 22, 2024
@llucax llucax modified the milestones: v1.0.0-rc900, 1.0.0-rc1000 Sep 2, 2024
@llucax llucax modified the milestones: v1.0.0-rc1000, v1.0.0-rc1100 Oct 21, 2024
@llucax llucax modified the milestones: v1.0.0-rc1100, v1.0.0-rc1200 Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
part:actor Affects an actor ot the actors utilities (decorator, etc.) part:power-management Affects the management of battery power and distribution type:tech-debt Improves the project without visible changes for users
Projects
Status: To do
Development

No branches or pull requests

2 participants