Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
179 changes: 179 additions & 0 deletions src/snc/agents/activity_rate_to_mpc_actions/fox_mpc.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
import numpy as np
import random
from typing import List, Optional, Tuple

from snc.agents.activity_rate_to_mpc_actions.action_mpc_policy import ActionMPCPolicy
import snc.agents.agents_utils as agents_utils
import snc.agents.hedgehog.policies.policy_utils as policy_utils
import snc.utils.snc_types as types


class FoxMpcPolicy(ActionMPCPolicy):
def __init__(self,
physical_constituency_matrix: types.ConstituencyMatrix,
buffer_processing_matrix: types.BufferMatrix,
mpc_seed: Optional[int] = None) -> None:
"""
Obtain feasible binary actions from activity rates with feedback on how many actions have
been performed so far for a given horizon. The actions are drawn from a probability
distribution that aims to match the activity rates after some horizon. The feedback allows
to adjust the distribution so that underperformed activities are emphasised. Feasible
actions refer to those that drain nonempty buffers. In the case that some action is
infeasible, then the corresponding resource performs other activity.
Comment on lines +17 to +22

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This docstring doesn't seem right.


:param physical_constituency_matrix: Constituency matrix from environment. We assume it has
orthogonal rows.
:param buffer_processing_matrix: Buffer processing matrix from environment.
:return: None.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing mpc_seed parameter.

"""
# Ensure that constituency_matrix has a single one per column.
assert agents_utils.has_orthogonal_rows(physical_constituency_matrix), \
"Physical constituency matrix must have orthogonal rows."
Comment on lines +30 to +31

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this required here? This is only needed when sampling from a distribution so we can easily allocate the distribution along each row.

super().__init__(physical_constituency_matrix, mpc_seed)

self.buffer_processing_matrix = buffer_processing_matrix
self.n_activities = buffer_processing_matrix.shape[1]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please use num_activities for consistency with the rest of the code.

self.activities_per_resource, self.num_activities_per_resource \
= self.get_ind_activities_per_resource(physical_constituency_matrix)
self.exit_activities = self.get_exit_activities(buffer_processing_matrix)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This info is already in the job generator in the CRW object as self.job_generator.exit_nodes, could that be passed in the constructor of the class?

self.activities_to_target_buffers, self.activities_to_source_buffers \
= self.get_target_and_source_buffers(buffer_processing_matrix,self.exit_activities)

Comment thread
tiavlovsky marked this conversation as resolved.
@staticmethod
def get_exit_activities(buffer_processing_matrix):
exit_activities = set()
for a in range(buffer_processing_matrix.shape[1]):
if np.all(buffer_processing_matrix[:,a] <= 0):
exit_activities.add(a)
return exit_activities

@staticmethod
def get_target_and_source_buffers(buffer_processing_matrix,exit_activities):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Docstring and type annotations missing.

n_activities = buffer_processing_matrix.shape[1]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please could you change the name to num_activities for consistency?

activities_to_target_buffers = {}
activities_to_source_buffers = {}
for a in range(n_activities):
act_vector = buffer_processing_matrix[:,a]
activities_to_source_buffers[a] = int(np.where(act_vector < 0)[0][0])
if a in exit_activities:
continue
activities_to_target_buffers[a] = int(np.where(act_vector > 0)[0][0])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will this work for supply resources?

return activities_to_target_buffers, activities_to_source_buffers

@staticmethod
def get_ind_activities_per_resource(physical_constituency_matrix: types.ConstituencyMatrix) -> \
Tuple[List[List[int]], List[int]]:
"""
Return the index of activities per resource and the total number of activities per resource.

:param physical_constituency_matrix: Constituency matrix from environment. We assume it has
orthogonal rows.
:return: (activities_per_resource, num_activities_per_resource):
- activities_per_resource: List of lists of activities per resource.
- num_activities_per_resource: List of number of activities per resource.
"""
assert agents_utils.has_orthogonal_rows(physical_constituency_matrix), \
"Physical constituency matrix must have orthogonal rows."

activities_per_resource = [] # type: List[List[int]]
num_activities_per_resource = [] # type: List[int]

for c in physical_constituency_matrix:
activities_c = np.nonzero(c)[0]
assert activities_c.size > 0

activities_per_resource += [activities_c.tolist()]
num_activities_per_resource += [activities_c.size]

return activities_per_resource, num_activities_per_resource
Comment on lines +64 to +88

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The function get_ind_activities_per_resource already exits in FeedbackStationaryFeasibleMpcPolicy. Please put the function in a file that can be accessed by both classes.


def obtain_actions(self, **kwargs) -> types.ActionProcess:
"""
This method implements the abstract method from super class `ActionMPCPolicy`.
It first gathers the feedback information namely the number of times each activity has to
be performed (i.e. 'sum_actions') and the current state ('state'). Then, it calls its own
method to return a single action vector.
Comment on lines +92 to +95

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is from another class.


:return: actions: binary indicator matrix with number of rows equal to the number of
non-idling activities (i.e. num of columns of the constituency matrix), and number of
columns equal to number of time steps to perform MPC.
Comment on lines +97 to +99

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This doesn't seem accurate either, as the output actions has been defined as a column vector.

"""
assert 'state' in kwargs, "Current state ('state') is required to be passed as parameter."

state = kwargs["state"]
x_eff = kwargs["x_eff"]
x_star = kwargs["x_star"]
print(state.ravel().astype(int),x_eff.ravel().astype(int),x_star.ravel().astype(int))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please use debug flags for print statements.

idling_set = kwargs["r_idling_set"]
draining_resources = kwargs["draining_resources"]
print(idling_set)
print(self.activities_per_resource)
Comment on lines +109 to +110

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please use debug flags for print statements.

x_target = x_star if len(idling_set) > 0 else x_eff
buffer_weights = list(np.maximum(x_target - state, 0).astype(int).ravel())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You only consider the amount of items here (back-pressure like). Would it be worthy to weight buffers based on draining time/rates and/or cost too?

print(buffer_weights)
Comment thread
sergiovalmac marked this conversation as resolved.
actions = np.zeros((self.n_activities,1))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We need to raise an exception if the passed num of MPC steps > 1.


for r in draining_resources:
actions_list = self._get_actions_list(r,state,buffer_weights,False)
#random.shuffle(actions_list)
decided_action,_ = actions_list[0]
actions[decided_action,0] = 1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
actions[decided_action,0] = 1
actions[decided_action, 0] = 1


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change


for r in range(len(self.activities_per_resource)):
if r in draining_resources:
continue
actions_list = self._get_actions_list(r,state,buffer_weights, r in idling_set)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Commas:

Suggested change
actions_list = self._get_actions_list(r,state,buffer_weights, r in idling_set)
actions_list = self._get_actions_list(r, state, buffer_weights, r in idling_set)

if not actions_list:
continue
random.shuffle(actions_list)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same comment as above, draw a random integer as index instead of reshuffling the whole list and taking the first element.

print(r,actions_list)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please use debug flags or logging instead of printing.

decided_action,_ = actions_list[0]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comma:

Suggested change
decided_action,_ = actions_list[0]
decided_action, _ = actions_list[0]


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Extra blank space

Suggested change


actions[decided_action,0] = 1
print()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
print()

return actions

def _get_actions_list(self,r,state,buffer_weights,r_in_idling_set):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Few things here:

  • Docstring missing.
  • Type annotations missing.
  • r is a cryptic variable name, better resource.
  • Blank spaces after commas missing.


actions_list = []
max_weight = 0

has_starved_activities = False
for a in self.activities_per_resource[r]:
if a in self.exit_activities:
continue
source_buffer = self.activities_to_source_buffers[a]
target_buffer = self.activities_to_target_buffers[a]
Comment on lines +147 to +148

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Are you taking into account that there could be multiple source and target buffers here?

buffer_weight = buffer_weights[target_buffer]
if buffer_weight > 0 and state[source_buffer,0] == 0:
has_starved_activities = True

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I understand that Lines 144-152 are needed to compute has_starved_activities. Could we embed them in a function that returns a bool?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is has_starved_activities used at all beyond the print statement? If this is only needed as a sanity check, could we include it at the end of this method with an assertion?


for a in self.activities_per_resource[r]:
if state[self.activities_to_source_buffers[a]] == 0:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is difficult to follow. Could we summarise this condition as a function with a descriptive name that returns a bool?

continue
if a in self.exit_activities:
source_buffer = self.activities_to_source_buffers[a]
buffer_weight = buffer_weights[source_buffer]
#weight = max(1,buffer_weight)
weight = max(1,int(state[source_buffer,0]))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Commas:

Suggested change
weight = max(1,int(state[source_buffer,0]))
weight = max(1, int(state[source_buffer, 0]))

if weight == max_weight:
actions_list.append((a,weight))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comma:

Suggested change
actions_list.append((a,weight))
actions_list.append((a, weight))

elif weight > max_weight:
actions_list = [(a,weight)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comma:

Suggested change
actions_list = [(a,weight)]
actions_list = [(a, weight)]

max_weight = weight
continue

source_buffer = self.activities_to_target_buffers[a]
target_buffer = self.activities_to_target_buffers[a]
buffer_weight = buffer_weights[target_buffer]
if r_in_idling_set and buffer_weight == 0:# and not has_starved_activities:
continue
if buffer_weight == max_weight:
actions_list.append((a,buffer_weight))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comma:

Suggested change
actions_list.append((a,buffer_weight))
actions_list.append((a, buffer_weight))

elif buffer_weight > max_weight:
actions_list = [(a,buffer_weight)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comma:

Suggested change
actions_list = [(a,buffer_weight)]
actions_list = [(a, buffer_weight)]

max_weight = buffer_weight
print(has_starved_activities)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Debug flag or logging?

Suggested change
print(has_starved_activities)

return actions_list
Loading