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

Model Directives error function mechanism should be a post-loading transform #14084

Closed
ggould-tri opened this issue Sep 17, 2020 · 5 comments · Fixed by #15949
Closed

Model Directives error function mechanism should be a post-loading transform #14084

ggould-tri opened this issue Sep 17, 2020 · 5 comments · Fixed by #15949

Comments

@ggould-tri
Copy link
Contributor

(this code was in dev at the time this issue was filed but is moving out shortly; this issue is to tag a TODO in that process)

The model directives mechanism for incorporating user-provided error is ugly (the caller supplies an error function which is invoked by AddWeld on each invocation).

Simply removing the error function mechanism would be incorrect: Modeling error is a core source of error that has been missing and a major motivation for bringing model directives to drake. A superior mechanism would be to postprocess the parsed model directives with a similar error function (though using names rather than frame objects).

To this end #14038 will move the error function to using names rather than frames. This should allow error injection to be migrated to a directives preprocessing step within ProcessModelDirectives in the future without any API or semantics changes:

  • As the first step of ProcessModelDirectives
  • Walk the model directives, expanding names
  • When an AddWeld is encountered that matches the error function, apply the error transform to the weld transform.
  • Remove the frame name check from the unit test.

This preprocessing step can then be factored out into a separate post-loading transform step:

  • Factor the code to handle the optional argument into a separate public function.
  • Call that function with the optional argument.
  • Deprecate the optional argument.
@ggould-tri ggould-tri self-assigned this Sep 17, 2020
@ggould-tri
Copy link
Contributor Author

Note that the ModelInstanceInfo structure might subsume some part of this mechanism. They serve related purposes, although that structure lacks the frame-level convenience of handling this in AddWeld.

@jwnimmer-tri
Copy link
Collaborator

FYI It's also possible that the ongoing work to use System Parameters on the plant would allow us to update weld joint offsets within the Context, which is even better and so the directives-time syntax for it would be unnecessary (or would simply feed into the default Parameter).

@sherm1
Copy link
Member

sherm1 commented Sep 17, 2020

Yes, I believe @joemasterjohn is working on that as we type.

@SeanCurtis-TRI
Copy link
Contributor

Team assigned based on past assignment of the porting work (#13282). Feel free to change it if incorrect.

@jwnimmer-tri
Copy link
Collaborator

We've stopped using ModelWeldErrorFunction in Anzu. We should deprecate it within Drake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants