-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Output KinematicsCache from RBP #3325
Comments
I am open to close this issue and re-open #3316. But let me see if I can share with you some thoughts. Example 1. RBP --> RbpTransalator --> BotVisualizer Example 2. SimplePendulum --> RbpTransalator --> BotVisualizer So, all the super-confusing components were already present in #3316. Actually here I am just simplifying to two classes instead of three. Again, I might be missing something. Let me know what you think. |
This is tagged as high priority; is that true? Is the only top-level goal to be able to write something like |
And a System 2.0 versions of |
Yes, that is why the high priority. It should be easy to implement once we agree on what exactly to implement. The high priority is because we at least need RBP, and BotVisuzlizer to simulate something and visualize it (as done with the Kuka demo). |
KinematicsCache is going to have a short lifetime since its functionality should be absorbed by System2's built-in caching facility. So I don't think we should be designing in any new dependence on it (unless this is meant as a short-term hack). Exposing meaningful information on output ports makes more sense and is more general since any System could output such data, while KinematicsCache is only available from Systems that have a RigidBodyTree embedded somewhere. |
Is it possible to incrementally transform the |
That will be much easier if we don't expose KinematicsCache outside RBTree and RBPlant. Then we can gradually move its contents in the Sys2 cache until it is empty. |
Sharing of |
Would it be too against the spirit of System 2.0 to have I hesitate to recommend such a design because it feels like it's a side-channel around System 2.0's port or cache abstractions. |
System 2 has its own nascent caching mechanism which should not be strangled in the womb by outputting some other caching mechanism! |
For future reference, I'll restate an example I gave on slack earlier: consider a robot with 20 accelerometers attached to it (we've actually discussed doing this for Valkyrie; this is not some contrived example). If every RigidBodyAccelerometer has to redo the work to compute the data currently stored in So what is the long-term vision for this? The discussion on slack seemed to be leaning towards only outputting a minimal representation of state (q and v for a RigidBodyPlant), and that cache is only an implementation detail, hidden to someone assembling a Diagram. That would imply that sharing cached results should be done 'underhand'. Instead, I'd like to argue that passing data between systems should always be done through ports; otherwise this nice, decoupled system design is in name only. So I'm in favor of @amcastro-tri's original plan of having a |
How about instead of having one output port that contains a Maybe |
The System 2 cache is contained inside the Context and is available to all subsystems for every computation performed at run time (in a carefully managed way that guarantees up to date results). In a sense it is always exported implicitly. It should never be exposed explicitly on an output port.
Agreed! That would never happen making proper use of the System 2 facilities. |
I wonder what that API would look like. Right now we can access subsystem contexts like done here for the PID controller. The access can be done if you have the pointer to the system which subcontext you want to access to. |
@sherm1, hmm, I mean it works, but the output ports of a I suppose maybe you're planning to require having access to the state vector in order to access the cache for a system (this is not what's currently implemented), but in that case, why not just have a scalar output port, outputting a hash value computed from the state? |
From @sherm1:
I agree as well, and for the same reasons. Of course, this would be clearer if the System2 cache facilities actually existed, so I will dust off that branch this week. It was previously PR #3135. We tabled it because there were competing priorities, and because @sherm1 and I have an unresolved design disagreement. That disagreement is tangential to this thread, where we entirely agree. From @tkoolen:
I absolutely, 100% agree with this. If we allowed
As an interim solution, I think this (or anything similar) is fine. It also gets at an important point. The System2 cache infrastructure will supersede the aspects of KinematicsCache that manage data lifetime. It will not supersede the aspects of KinematicsCache that are a notation for the result of particular, expensive computations about RigidBodyTree. We will still need that notation, to be stored in System2 cache, and to be transmitted on a |
Sigh. My post crossed the most recent from @amcastro-tri and @tkoolen in flight. I do not think @sherm1 meant to claim that multiple subsystems would share a cache. If he did, then I strongly disagree, for the reasons @tkoolen mentioned! I believe Sherm meant, and I also would assert, that (a) each subsystem always has its own private cache available and (b) cache entries which depend on a given input are invalidated whenever that input changes. So, a downstream consumer of KinematicsCache can check whether its KinematicsCache input has changed, and redo dependent computations only if it hasn't. |
Right! What I meant is that the full cache is always lurking behind every computation within a Diagram so that (as managed carefully via input and output ports) all needed information can be obtained without (a) passing caches around, (b) recomputing anything that has already been computed, or (c) computing things that don't get used. Each subsystem sees only its own cache, but the framework as a whole has the Diagram perspective and can efficiently deliver computations between subsystems via the ports. |
OK, that makes sense. Back to the original issue :-). What ports do you think should be added to RigidBodyPlant to enable efficient RigidBodySensors and visualizers? |
fyi - this is also blocking the valkyrie sim. |
To get the conversation started how about let's make it output the following at least initially? We can always add more output ports with additional data as necessary.
|
@liangfok asked me to comment on this @liangfok, i think that the conceptual output of the rigid body plant is very clear -- it should contain the generalized state of the plant (which is positions and velocities). in my view most of the discussion here is about passing the result of computations on that state (transforms, accelerations, etc) simply to make downstream computations more efficient for sensors, etc. |
Yes, the additional state that we were thinking about outputting from I believe one of the original instigators that resulted in this discussion is #3228 (comment), where it was pointed out that In #3325 (comment), @tkoolen makes a strong argument for why efficiency is important with respect to supporting sensors. In System 1.0, the |
@jwnimmer-tri, I'm aware of that. My reasons for not liking fat outputs so much are outlined here: #3325 (comment). But fat output ports certainly are a valid solution to this issue; just a matter of preference. |
One option is to initially provide both fat ports and small ports, then over time we can see if both are in fact useful for different purposes. |
I believe mine is the same as @david-german-tri's: we have a beautiful caching mechanism designed for System 2 that is completely sufficient for efficiently and correctly connecting subsystems via input/output ports. So my stance is that this is a non-issue. The cache should be invisible and the solutions should involve designing useful output ports that expose meaningful data. |
I was able to chat about this quickly with @RussTedrake and @tkoolen this afternoon. We cleared up a few misunderstandings. (Please correct me if I misrepresent anything!) First, Russ confirmed that it's not necessarily a requirement that Second, Russ pointed out a couple of additional requirements that I hadn't thought of. (a) It should be possible to have a network boundary between That suggests an implementation spiral like the following. My primary interest is in the application of the System2 architecture; if I've missed some dynamics details please accept my apologies! step 0 (fat ports, no cache). step 1 (fat ports, cache). Once the System2 cache is available, KC/RBC can be stored there (and invalidated on changes to x or u) instead of recomputed on every call to EvalOutput. step 2, option a (thin ports, cache). Once #2890 is resolved, it may be worthwhile to break up KC/RBC into smaller outputs. Then the step 2, option b (no ports, cache). You could alternatively glom RigidBodyPlant and the sensors into a single System with a shared cache. I don't think anyone sees a good reason to prefer this, but the door wouldn't be closed to it. |
Just going to add a couple of things I took away from the meeting. Let me know if I failed to understand something. But first:
How about @david-german-tri and I were initially confused about how @RussTedrake wanted to 1) output only q and v from So in a sense, Consumers of a On a somewhat related note, I think we should move towards not exposing the
Our reasoning for separating out contact forces is that they may depend on the |
Making parts of an output mutable would be a complete end-around the System 2 caching system, identical to exporting the KinematicsCache. We should not do that. If we don't have suitable caching available yet, we can just have the RBPlant do extra work to make sure everything it outputs is valid. BTW, I agree that caching is an appropriate use of |
So, basically, Why not have downstream consumers of it simply make a mutable copy of the |
@sherm1, thinking about networking is what convinced me the approach we came up with at the meeting is the right way to go. Consider this: how would you send a If you choose the minimal LCM message, you could have the
@liangfok, that's an option, but my argument is that the input will remain const in the way that matters anyway (logically const), even if it's not bitwise const. |
SGTM. Although I agree with @sherm1 that it's a bad idea, it sounds like we should also track an option 2c. Importantly, we don't have to choose among the 2x's to do steps 0 and 1, which will unblock most real use cases. step 2, option c (fat port, separate caching). Make the ancillary computation results in the KC/RBPS
👍 to this part. I knew I was forgetting something. |
Just step 0 already does this, and it will even make the non-networked case be as efficient as possible: computing the dynamics in |
I totally agree and this is how the System 2 caching mechanism should behave. Surprisingly this is somewhat controversial at this point but I hope we'll resolve it soon. |
The controversial thing is sharing cache that is supposed to be private to a subsystem implicitly (i.e., not through output ports) with other subsystems in a diagram (e.g., #3325 (comment)). The proposal here is to associate cache with the output of a subsystem, not with the subsystem itself, so that it passes through an output port, and the dependencies are explicit. Maybe I completely misunderstood, but I was under the impression that the current cache design only associates cache with a subsystem. |
The point is that in a networked situation, the |
If it helps: another architecture for the networked plant state is to transmit the minimal representation, and then have an explicit System on the receiving side that takes minimal representation as input and provides the supplemental representation as (cached) output. Then you get minimalism, caching, explicit documentation of the computations' structure, and it would (also) be reusable code pattern if you had different plants with different supplemental state. |
@jwnimmer-tri beat me to it. But this whole argument is 2a vs 2c and can wait. |
Completely agree. |
@jwnimmer-tri, yep, exactly what I meant in #3325 (comment) with
|
It sounds like I should abstain from this discussion for the time being 😄. I am happy to talk though the computation / caching for the future numbered items if and when we need them, if anyone wants. |
Sorry, didn't mean to scare you away, haha :-) |
I'm good. It's an interesting discussion, just not a problem I need to own. |
Caching is supposed to apply to output ports as well (and also other computations like derivatives, discrete updates, energy calculations, and anything a concrete System may want to add besides the general API). Cached output ports provide a disciplined way to allow caching to reach downstream from the System that owns that piece of cache real estate, without having to expose any of the internal workings (even the fact that a cache exists at all should be evident only through performance). |
Seeing as there is still no consensus on this issue (given #3471), I propose clearing things up with a meeting, this time with everybody involved in this issue. |
Please consider that accessing an output port triggers the associated computation (that is, the output port's value is guaranteed to be up to date when you see it). Consequently output ports need to be segregated appropriately; asking for body poses should not trigger acceleration computations. Also, a port that includes accelerations is necessarily a direct feedthough from the RBPlant's actuator inputs. That is not what you want from a port that returns kinematics, which is not direct feedthrough and could thus serve as input to a sensor System whose output goes to a controller that generates actuation inputs to RBPlant. Returning KinematicsCache as a single output port thus implies that everything in it gets calculated at once. That likely means we have to include all the sensor and controller code in RBPlant rather than using System interconnections. That could be done but seems like a complete end-around the System 2 framework. IMO, a nice solution would involve at least 3 output ports:
It is not clear to me that we should be exporting internal quantities through output ports (rather than bespoke RBPlant methods) but if we do those would be good candidates for additional ports so that expensive computations (mass matrix, for example) could be delayed unless someone asks for them. (Accelerations can be calculated without forming a mass matrix.)
That seems like a good idea! |
The above seems to be a strong argument for using "thin" ports. I did not realize that the act of accessing a port triggers the associated computation, though it sounds like a really nice optimization. I'm now thinking about the implications of this fact on the implementation of the system that owns the output port. If a particular output port is accessed only 50% of the simulation cycles, is it the responsibility of the system that owns the output port to only compute the value supplied on that port 50% of the time? |
The dataflow model (a.k.a. lazy evaluation) that we're now using never computes anything unless someone asks for it. So an output port that gets accessed occasionally only gets evaluated occasionally. The same scheme works for all the rest of the computations also, like derivatives, discrete updates, energy calculations, initial conditions, and so on. We don't know how often those need to be computed until someone asks. |
Not on master yet - that's #3455. (pending your review) |
Per discussion in #3316, we need to output the
KinematicsCache
as an output forBotVisualizer
not having to recomputed already available quantities in theKinematicsCache
.The output will be in the form of an
AbstractValue
andBotVisualizer
will have an input port for it.For systems like a simple pendulum dynamics but that we want to render as a three dimensional multibody system, we will just leave the
KinematicsCache
port unconnected case in which theBotVisualizer
will just instantiate its ownKinematicsCache
as it is done right now. Therefore we would need the capability to leave inputs unconnected and forSystem<T>
's to query whether these are connected or not.Two questions arise @david-german-tri and @sherm1:
System<T>
method to check whether an input port is connected or not? likeSystem<T>::is_input_port_conneted(port_number)
.The text was updated successfully, but these errors were encountered: