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

Introducing PartialOrder #288

Open
wants to merge 33 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
0aca75d
introducing Plan
beverlylytle Apr 1, 2021
da4fe74
lint
beverlylytle Apr 1, 2021
b7d8835
docstrings, inherit from compas.base.Base, safety checks
beverlylytle Apr 6, 2021
1c92c81
add tests, and some fixes
beverlylytle Apr 8, 2021
031e3a9
more tests
beverlylytle Apr 8, 2021
87c1a67
Update src/compas_fab/datastructures/plan.py
beverlylytle Apr 8, 2021
6824321
Update src/compas_fab/datastructures/plan.py
beverlylytle Apr 8, 2021
ed26c84
Update src/compas_fab/datastructures/plan.py
beverlylytle Apr 8, 2021
b79cb47
add alias for ironpython
beverlylytle Apr 8, 2021
f44387f
more ipy aliases
beverlylytle Apr 8, 2021
e00f63c
Update CHANGELOG.rst
beverlylytle Apr 12, 2021
e5a685a
Use compas.json_<load/dump>
beverlylytle Apr 12, 2021
164a68d
Inherit from Base rather than Datastructure
beverlylytle Apr 12, 2021
a88c502
remove extraneous from_data
beverlylytle Apr 12, 2021
ccbca4b
Add module level docstring
beverlylytle Apr 12, 2021
55ee2e8
Merge branch 'main' into dag
gonzalocasas Apr 12, 2021
2adbd02
Merge branch 'dag' of https://github.com/compas-dev/compas_fab into dag
beverlylytle Apr 12, 2021
d89ee63
use compas.datastructures.Graph
beverlylytle Apr 15, 2021
93f3952
locally import networkx to make ipy happy
beverlylytle Apr 15, 2021
9651875
fix changelog
beverlylytle Apr 15, 2021
f05da6f
extend test
beverlylytle Apr 15, 2021
8c8d637
better test
beverlylytle Apr 16, 2021
d4941f5
update to newest compas version
beverlylytle Apr 20, 2021
744ccba
Merge branch 'main' into dag
beverlylytle May 25, 2021
4fcb952
Rename Plan to PartialOrder
beverlylytle May 25, 2021
829926c
Rename more stuff
beverlylytle May 25, 2021
ab10364
Change test to make ironpython happy
beverlylytle May 26, 2021
23ae475
Add wikipedia link
beverlylytle May 26, 2021
07268fd
Added first draft of accept methods
beverlylytle May 26, 2021
a5341ad
Yeah, I remember how my code works
beverlylytle May 26, 2021
d28fb49
Update src/compas_fab/datastructures/partial_order.py
beverlylytle May 26, 2021
31b963a
Merge branch 'main' into dag
beverlylytle May 27, 2021
a9e96ab
Merge branch 'dag' of https://github.com/compas-dev/compas_fab into dag
beverlylytle May 27, 2021
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
1 change: 1 addition & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Unreleased
**Added**

* Added support for MoveIt on ROS Noetic
* In`compas.datastructures`, added `Plan`, `PlannedAction`, `Action` and `IntegerIdGenerator`
beverlylytle marked this conversation as resolved.
Show resolved Hide resolved

**Changed**

Expand Down
16 changes: 16 additions & 0 deletions src/compas_fab/datastructures/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
from .plan import (
gonzalocasas marked this conversation as resolved.
Show resolved Hide resolved
Action,
DependencyIdException,
IntegerIdGenerator,
Plan,
PlannedAction
)


__all__ = [
'Action',
'DependencyIdException',
'IntegerIdGenerator',
'Plan',
'PlannedAction',
]
368 changes: 368 additions & 0 deletions src/compas_fab/datastructures/plan.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,368 @@
from __future__ import absolute_import
from __future__ import division
from __future__ import print_function

import json
import threading
from collections import OrderedDict
from itertools import count

from compas.base import Base
from compas.datastructures import Datastructure
from compas.utilities import DataDecoder
from compas.utilities import DataEncoder


__all__ = [
'Action',
'DependencyIdException',
'IntegerIdGenerator',
'Plan',
'PlannedAction',
]


class IntegerIdGenerator(Base):
"""Generator object yielding integers sequentially in a thread safe manner.

Parameters
----------
start_value : :obj:`int`
First value to be yielded by the generator.
"""
def __init__(self, start_value=1):
super(IntegerIdGenerator, self).__init__()
self.last_generated = start_value - 1
self._lock = threading.Lock()
self._generator = count(start_value)

def __next__(self):
with self._lock:
self.last_generated = next(self._generator)
return self.last_generated

@property
def data(self):
return {
'start_value': self.last_generated + 1
}

def to_data(self):
return self.data

@classmethod
def from_data(cls, data):
return cls(data['start_value'])

@classmethod
def from_json(cls, filepath):
with open(filepath, 'r') as fp:
beverlylytle marked this conversation as resolved.
Show resolved Hide resolved
data = json.load(fp, cls=DataDecoder)
return cls.from_data(data)

def to_json(self, filepath, pretty=False):
with open(filepath, 'w+') as f:
if pretty:
json.dump(self.data, f, sort_keys=True, indent=4, cls=DataEncoder)
else:
json.dump(self.data, f, cls=DataEncoder)


class DependencyIdException(Exception):
"""Indicates invalid ids given as dependencies."""
def __init__(self, invalid_ids, pa_id=None):
message = self.compose_message(invalid_ids, pa_id)
super(DependencyIdException, self).__init__(message)

@staticmethod
def compose_message(invalid_ids, pa_id):
if pa_id:
return 'Planned action {} has invalid dependency ids {}'.format(pa_id, invalid_ids)
return 'Found invalid dependency ids {}'.format(invalid_ids)


class Plan(Datastructure):
Copy link
Member

Choose a reason for hiding this comment

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

Here I have a big question: why not inherit from the compas Graph instead? Otherwise, we will definitely duplicate features. As an example, I would almost immediately need to be able to turn a plan back and forth from a NetworkX DiGraph, and that works with Graph but not if the object inherits directly from Datastructure. The alternative would be containment of a Graph, but I am not sure that'd be very clean.

"""Data structure for holding the information of a partially ordered plan
(a directed acyclic graph). The content of any event of the plan is contained
in an :class:`compas_fab.datastructures.Action`. An event is scheduled and
added to the plan through a :class:`compas_fab.datastructures.PlannedAction`.
The dependency ids of a planned action can be thought of as pointers to the
parents of that planned action.

Parameters
----------
planned_actions : list of :class:`compas_fab.datastructures.PlannedAction`
The planned actions will be stored as an ordered dictionary, so their
ids should be distinct. Defaults to an empty list.
id_generator : Generator[Hashable, None, None]
Copy link
Member Author

Choose a reason for hiding this comment

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

Unsure about the typing here, and how sphinx wants it...

Object which generates keys (via ``next()``) for
:class:`compas_fab.datastructures.Action`s added using this object's
methods. Defaults to :class:`compas_fab.datastructures.IntegerIdGenerator`.
"""
def __init__(self, planned_actions=None, id_generator=None):
super(Plan, self).__init__()
planned_actions = planned_actions or []
self.planned_actions = planned_actions
if id_generator is None:
try:
start_value = max(self.planned_actions.keys()) + 1 if self.planned_actions else 1
except Exception:
raise Exception('Given ids not compatible with default id_generator.')
id_generator = IntegerIdGenerator(start_value)
self._id_generator = id_generator

@property
def planned_actions(self):
return self._planned_actions

@planned_actions.setter
def planned_actions(self, planned_actions):
self._planned_actions = OrderedDict({pa.id: pa for pa in planned_actions})
self.check_all_dependency_ids()

def plan_action(self, action, dependency_ids):
"""Adds the action to the plan with the given dependencies,
and generates an id for the newly planned action.

Parameters
----------
action : :class:`comaps_fab.datastructures.Action`
The action to be added to the plan.
dependency_ids : set or list
The keys of the already planned actions that the new action
is dependent on.

Returns
-------
The id of the newly planned action.
"""
self.check_dependency_ids(dependency_ids)
action_id = self._get_next_action_id()
planned_action = PlannedAction(action_id, action, dependency_ids)
self.planned_actions[action_id] = planned_action
return action_id

def append_action(self, action):
Copy link
Member

Choose a reason for hiding this comment

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

I find the pair plan_action and append_action not very intuitive as being a pair of related functions.
Why not merge them both into add_action, if the dependency_ids param is none/empty, then do the appending logic?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to make a bigger distinction between dependency_ids=None and dependency_ids=[] because they result in very different behavior and I can foresee that being the source of annoying bugs for the user.

"""Adds the action to the plan dependent on the last action added
to the plan, and generates an id for this newly planned action.

Parameters
----------
action : :class:`comaps_fab.datastructures.Action`
The action to be added to the plan.

Returns
-------
The id of the newly planned action.
"""
dependency_ids = set()
if self.planned_actions:
last_action_id = self._get_last_action_id()
dependency_ids = {last_action_id}
return self.plan_action(action, dependency_ids)

def remove_action_by_id(self, action_id):
"""Removes the action with the given id from the plan and all its
dependencies.

Parameters
----------
action_id : Hashable
beverlylytle marked this conversation as resolved.
Show resolved Hide resolved
Id of the planned action to be removed.

Returns
-------
:class:`compas_fab.datastructure.Action`
The action of the removed :class:`compas_fab.datastructure.PlannedAction`
beverlylytle marked this conversation as resolved.
Show resolved Hide resolved
"""
planned_action = self.planned_actions.pop(action_id)
for pa in self.planned_actions.values():
pa.dependency_ids.discard(action_id)
return planned_action.action

def _get_last_action_id(self):
last_action_id, last_action = self.planned_actions.popitem()
self.planned_actions[last_action_id] = last_action
return last_action_id

def _get_next_action_id(self):
return next(self._id_generator)

def check_dependency_ids(self, dependency_ids, planned_action_id=None):
"""Checks whether the given dependency ids exist in the plan.

Parameters
----------
dependency_ids : set or list
The dependency ids to be validated.
planned_action_id : Hashable
The id of the associated planned action. Used only in
the error message. Defaults to ``None``.

Raises
------
:class:`compas_fab.datastructures.DependencyIdException`
"""
dependency_ids = set(dependency_ids)
if not dependency_ids.issubset(self.planned_actions):
invalid_ids = dependency_ids.difference(self.planned_actions)
raise DependencyIdException(invalid_ids, planned_action_id)

def check_all_dependency_ids(self):
"""Checks whether the dependency ids of all the planned actions
are ids of planned actions in the plan.

Raises
------
:class:`compas_fab.datastructures.DependencyIdException`
"""
for pa_id, planned_action in self.planned_actions.items():
self.check_dependency_ids(planned_action.dependency_ids, pa_id)

def check_for_cycles(self):
""""Checks whether cycles exist in the dependency graph."""
self.check_all_dependency_ids()

def helper(cur, v, r):
v[cur] = True
r[cur] = True
for dep_id in self.planned_actions[cur].dependency_ids:
if not v[dep_id]:
helper(dep_id, v, r)
elif r[dep_id]:
raise Exception("Cycle found with action ids {}".format([pa_id for pa_id, seen in r.items() if seen]))
r[cur] = False

visited = {pa_id: False for pa_id in self.planned_actions}
rec_dict = {pa_id: False for pa_id in self.planned_actions}

for pa_id in self.planned_actions:
if not visited[pa_id]:
helper(pa_id, visited, rec_dict)

def linear_sort(self):
"""Sorts the planned actions linearly respecting the dependency ids.

Returns
-------
:obj:`list` of :class:`compas_fab.datastructure.Action`
"""
self.check_for_cycles()

def helper(s, v, cur_id):
v.add(cur_id)
action = self.planned_actions[cur_id]
for dep_id in action.dependency_ids:
if dep_id not in v:
helper(s, v, dep_id)
s.append(action)

stack = []
visited = set()

for action_id in self.planned_actions:
if action_id not in visited:
helper(stack, visited, action_id)

return stack

@property
def data(self):
return dict(
planned_actions=list(self.planned_actions.values()),
id_generator=self._id_generator,
)

@data.setter
def data(self, data):
self.planned_actions = data['planned_actions']
self._id_generator = data['id_generator']

@classmethod
def from_data(cls, data):
return cls(**data)
beverlylytle marked this conversation as resolved.
Show resolved Hide resolved


class PlannedAction(Datastructure):
beverlylytle marked this conversation as resolved.
Show resolved Hide resolved
"""Represents an action which has been scheduled in a plan.

Parameters
----------
action_id : Hashable
An identifier of the action. Used by other actions and the plan
it is associated with.
action : :class:`compas_fab.datastructures.Action`
The action to be planned.
dependency_ids : set or list
The ids of the actions upon which `action` is dependent.
"""
def __init__(self, action_id, action, dependency_ids):
super(PlannedAction, self).__init__()
self.id = action_id
self.action = action
self.dependency_ids = dependency_ids

@property
def dependency_ids(self):
return self._dependency_ids

@dependency_ids.setter
def dependency_ids(self, value):
self._dependency_ids = set(value)

def __str__(self):
return 'PlannedAction<id={}, action={}>'.format(self.id, self.action)

@property
def data(self):
return dict(
action_id=self.id,
action=self.action,
# sets are not json serializable
Copy link
Member

Choose a reason for hiding this comment

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

A message for our future selves... ;)

dependency_ids=list(self.dependency_ids),
)

@data.setter
def data(self, data):
self.id = data['action_id']
self.action = data['action']
self.dependency_ids = data['dependency_ids']

@classmethod
def from_data(cls, data):
return cls(**data)


class Action(Datastructure):
"""Abstract representation of an event independent of its timing.

Parameters
----------
name : :obj:`str`
The name of the action.
parameters : dict
Any other content associated to the action housed in key-value pairs.
"""
def __init__(self, name, parameters=None):
super(Action, self).__init__()
self.name = name
self.parameters = parameters or {}

def __str__(self):
return 'Action<name={}>'.format(self.name)

@property
def data(self):
return dict(
name=self.name,
parameters=self.parameters,
)

@data.setter
def data(self, data):
self.name = data['name']
self.parameters = data['parameters']

@classmethod
def from_data(cls, data):
beverlylytle marked this conversation as resolved.
Show resolved Hide resolved
return cls(**data)
Loading