plasmodic / ecto

ecto is a dynamically configurable Directed Acyclic processing Graph (DAG) framework.
http://ecto.willowgarage.com/
BSD 3-Clause "New" or "Revised" License
98 stars 37 forks source link

Directed graph configuration #260

Closed stonier closed 10 years ago

stonier commented 10 years ago

Currently the configuration process that occurs when you do configure_all is an unsorted process. I would like to push this through a directed graph, so the results of one cell's configuration can be transferred to the next so that it can configure itself appropriately.

The current workaround is to execute such a directed graph once, which is somewhat awkward, because it now means I have cells hanging around for the whole execution whose only purpose is for configuration AND I have to have branching code in the process call which checks to see if variables have been initialised or not.

e.g. I would like to have a cell which accepts a yaml string that converts to a camera info structure. This yaml string can come from various places depending on the program. Some example cell configurations:

# A testing configuration when I have the yaml configuration locally
loadYaml -> camerainfo
# gets the yaml string as a package from across a socket connection to the image board
image_bridge -> camerainfo
# A useful configuration when running a bag
rosparam2yaml -> camerainfo

Even the camera info cell itself is only needed at configuration time - doesn't need to hang around for execution.

So...directed configuration? I wonder if this was originally intended. I noticed that the incoming and outgoing tendrils as part of the configure function arguments are currently just dangling and good for nothing. So I suspect this is something that was originally planned.

Anyway, code for such a directed configuration is in this pull request. Has minor modifications - most important points:

stonier commented 10 years ago

Hmm, this needs some work still - it imght break alot of existing configurations that do not have tendril values configured at the configure step. i.e. you see this error when calling configure_all on a blackbox which hasn't itself internally configured required ins and outs:

Traceback (most recent call last):
  File "./dslam", line 109, in <module>
    configuration_plasm.configure_all()
ecto.CellException:            exception_type  CellException
[cell_name] = Beachhead

[type] = boost::exception_detail::clone_impl<ecto::except::ValueNone>

[what] = ValueNone

[when] = Copying stereo_parameters to stereo_parameters

So...question is whether to create a variable for the plasm to enable directed sorting (with default value of False), or to go to all the effort of updating old cells to properly conform. I'd say the former, but I think I would have no use from now on with unsorted configuration.

vrabaud commented 10 years ago

you also need to rebase your PR. Also, do you run the tests ? They are fairly extensive. Thx for all your work ! Also, did you check out the gooey ? (the web GUI)

stonier commented 10 years ago

Just spent the weekend getting familiarised with the tests because of this PR (one chunk at a time - there's an awful lot to get up to speed with in ecto!).

stonier commented 10 years ago

The gui is this one: https://github.com/plasmodic/ecto_gooey?

I hadn't checked it out yet - usually web gui's are the last thing I try. I'll give it a whirl though.

stonier commented 10 years ago

Just to confirm. This PR is pending a thorough test...don't merge yet!

vrabaud commented 10 years ago

I guess a rebase iw now needed here :)

stonier commented 10 years ago

Yup...I'm rewinding to catch up on a few things. Directed configuration has been working brilliantly for me for a few weeks, so I'll shape it up, checks tests and get it shipped shortly.

stonier commented 10 years ago

Reconstructing this pull request a bit more formally in #264.