The FRBC planner by VladIftime · Pull Request #5 · FlexMeasures/flexmeasures-s2

VladIftime · 2025-01-31T19:19:04Z

This is a big PR. So, I suggest a collaborative review.

Signed-off-by: F.N. Claessen <felix@seita.nl>

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

jorritn · 2025-02-06T11:04:49Z

flexmeasures_s2/profile_steering/common/joule_profile.py

Looks good. Seems to me this should work for a first version. There are some important differences with the original implementation. Two that quickly occur to me are: you allow None values is the profile list, and when checking for compatibility you only look at the time step duration and not the start time. This gives more flexibility but also enlarges the chance for bugs. Just something to consider.

jorritn · 2025-02-06T11:29:02Z

flexmeasures_s2/profile_steering/common/proposal.py

+        if self.cost_improvement_value is None:
+            self.cost_improvement_value = self.get_cost(
+                self.old_plan, self.global_diff_target
+            ) - self.get_cost(self.proposed_plan, self.global_diff_target)


Apparently it is. Sorry for bringing it up.

jorritn · 2025-02-06T11:15:40Z

flexmeasures_s2/profile_steering/common/proposal.py

Seems ok to me

jorritn · 2025-02-06T11:16:47Z

flexmeasures_s2/profile_steering/common/target_profile.py

Price target not yet implemented but I guess that's deliberately?

jorritn · 2025-02-06T11:18:49Z

flexmeasures_s2/profile_steering/frbc/device_planner/device_planner.py

Just as a suggestion: abstract base classes can be used here.

jorritn

Awesome work! Most of the porting has been done. After porting S2FrbcDeviceStateWrapper and the ProfileMetadata (or something equivalent), you should be good to run a test on find_best_plan(). The DevicePlan class doesn't have to be ported for that.

jorritn · 2025-02-06T11:21:04Z

flexmeasures_s2/profile_steering/frbc/device_planner/frbc/s2_frbc_device_controller.py

Is a duplicate with s2-frbc_device_planner. This can be removed, also because the device_controller is a different component in the architecture.

jorritn · 2025-02-06T11:21:59Z

flexmeasures_s2/profile_steering/frbc/device_planner/frbc/s2_frbc_device_planner.py

Why not extend the DevicePlanner class?

Apart from that. Looks very good!

jorritn · 2025-02-06T11:25:25Z

flexmeasures_s2/profile_steering/frbc/device_planner/frbc/s2_frbc_device_planner.py

+        return self.latest_plan.get_energy()
+
+    def accept_proposal(self, accepted_proposal):
+        if accepted_proposal.get_origin() != self:


origin needs to be added to the proposal class

jorritn · 2025-02-06T11:29:02Z

flexmeasures_s2/profile_steering/common/proposal.py

+        if self.cost_improvement_value is None:
+            self.cost_improvement_value = self.get_cost(
+                self.old_plan, self.global_diff_target
+            ) - self.get_cost(self.proposed_plan, self.global_diff_target)


Apparently it is. Sorry for bringing it up.

jorritn · 2025-02-06T11:36:10Z

flexmeasures_s2/profile_steering/frbc/device_planner/frbc/s2_frbc_device_state_wrapper.py

I would need some extra time to dive thoroughly into this but it looks good at first sight.

The other classes had type hints. Would be good to add it here too.

jorritn · 2025-02-06T16:26:37Z

flexmeasures_s2/profile_steering/root_planner.py

I'll do this one later

jorritn · 2025-02-06T16:28:07Z

flexmeasures_s2/scheduler/s2_frbc_device_state.py

If guess this needs some extension later. If I understand it correct, the storageStatus should be included, for example.

jorritn · 2025-02-06T16:39:45Z

flexmeasures_s2/profile_steering/frbc/operation_mode_profile_tree.py

+            for frbc_state in final_states:
+                frbc_state.generate_next_timestep_states(next_timestep)
+        end_state = self.find_best_end_state(
+            last_timestep.get_final_states_within_fill_level_target()


Does this include the emergency state? If yes, then it's fine.

Cool! It does 👍

jorritn · 2025-02-06T16:50:57Z

flexmeasures_s2/profile_steering/frbc/frbc_state.py

+            S2FrbcDeviceStateWrapper.get_leakage_rate(self.timestep, self.fill_level)
+            * seconds
+        )
+        self.fill_level += self.timestep.get_forecasted_usage()


I am not 100% sure whether this should be a plus or a minus. The original implementation has plus as well but intuitively I would think it should be a minus.

I'll check it

Intuition was right: the protocol spec says: A positive value indicates that the fill level will decrease due to usage. https://github.com/flexiblepower/s2-ws-json/blob/b831b6f1f19f824f32269ecd6ac6a6e250e7af80/s2-asyncapi/s2-cem.yaml#L599C131-L599C205

jorritn · 2025-02-06T16:54:04Z

flexmeasures_s2/profile_steering/frbc/usage_forecast_util.py

In response to your TODO: conceptually is the same as the FRBCUsageForecast. The utility function from_storage_usage_profile converts it from an S2 data object into an array.

…a couple of wrappers Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

…rrors Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

… a target as input Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

… added Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

This reverts commit f0e31b4.

* refactor: collect parameters Signed-off-by: F.N. Claessen <felix@seita.nl> * refactor: get rid of UUID actuator_id (prefer str instead) Signed-off-by: F.N. Claessen <felix@seita.nl> * delete: remove empty results file Signed-off-by: F.N. Claessen <felix@seita.nl> * docs: add instructions for profiling Signed-off-by: F.N. Claessen <felix@seita.nl> * refactor: get rid of UUID operation_mode_is (prefer str instead) Signed-off-by: F.N. Claessen <felix@seita.nl> --------- Signed-off-by: F.N. Claessen <felix@seita.nl>

Flix6x and others added 6 commits January 20, 2025 14:08

feat: add stubs for tests and initial code structure

fc02695

Signed-off-by: F.N. Claessen <felix@seita.nl>

Joule profile for target profile and original data

8548047

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Added the device state as ported from Java

c9468dc

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Added serialised JSON for FRBC device state

e0694ca

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Added the target joule profile

20ef8c8

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Created the first version of the python version

05bd40e

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

VladIftime requested a review from jorritn January 31, 2025 19:19

Added the congestion point and root planner and a Device Planner class

85c98f9

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

jorritn reviewed Feb 6, 2025

View reviewed changes

flexmeasures_s2/profile_steering/common/proposal.py

Copy link

Collaborator

jorritn Feb 6, 2025

Choose a reason for hiding this comment

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

Seems ok to me

jorritn reviewed Feb 6, 2025

View reviewed changes

flexmeasures_s2/profile_steering/common/target_profile.py

Copy link

Collaborator

jorritn Feb 6, 2025

Choose a reason for hiding this comment

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

Price target not yet implemented but I guess that's deliberately?

jorritn reviewed Feb 6, 2025

View reviewed changes

jorritn requested changes Feb 6, 2025

View reviewed changes

VladIftime added 11 commits February 7, 2025 13:22

Chore: type hints for easier debugging; removed profile insight, add …

b95a3a9

…a couple of wrappers Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Created pytest for FRBC

82900f2

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Adding unit test

10c28e5

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Profile_metadata

108c0a8

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Profile_metadata

de03441

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Fixed all errors

e29b40d

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

test returns null joule profile but it returns

4731690

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Created more robust testing. Still fails.

f305431

Need to discuss the change to the s2-python FRBC library

b59e50e

Need to discuss FRBC change in s2-python

5f17892

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Need to discuss FRBC change in s2-python

7f5c99c

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Flix6x mentioned this pull request Feb 18, 2025

Test calling the scheduler #4

Open

7 tasks

VladIftime added 4 commits February 19, 2025 12:40

Problems at the final generation step

4d593f9

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

The algorithm works with slight deviations probably due to rounding e…

6b2dbd4

…rrors Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

The algorithm works

a0ada05

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Chore:removed prints and files used for debuggin

4d2eedb

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

VladIftime and others added 12 commits March 14, 2025 11:19

Created planning service implementation that takes a device state and…

a0915b9

… a target as input Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Plan is being created

2fbda15

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Planning service provides initial plan

d335f6a

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Added cluster plan and global target

047e796

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Root and cluster point produce a improved plan close to the target.

db6728a

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

The Planning schedule works right now. The profiling resutls are also…

102a239

… added Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Scheduling 10 EVs

dad9e94

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Scheduling 10 EVs

f0e31b4

Signed-off-by: Vlad Iftime <vladiftime60@gmail.com>

Revert "Scheduling 10 EVs"

7345e28

This reverts commit f0e31b4.

Removed simple getters that get lost

f8739c3

Removed simple getters that get lost

c063972

Comments

Conversation

VladIftime commented Jan 31, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorritn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorritn Feb 6, 2025 • edited by Flix6x Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jorritn Feb 6, 2025 • edited by Flix6x Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jorritn Feb 6, 2025 •

edited by Flix6x

Loading

jorritn Feb 6, 2025 •

edited by Flix6x

Loading