RSDK-1899 rearchitect backend of constraint system in anticipation of new constraints being added #2108
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
viambot
added
the
safe to test
biotinker
commented
Mar 24, 2023
biotinker
commented
Mar 24, 2023
viambot
added
safe to test
|
@tessavitabile Follow-up from 1:1, the Important section at the bottom of Peter's PR description is the preparatory content for 2D Motion Planning (and eventually can lead to GeoPoint goals, other spatial goal encodings, etc.) |
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
spieswl
reviewed
Mar 27, 2023
spieswl
reviewed
Mar 27, 2023
spieswl
reviewed
Mar 27, 2023
motionplan/plannerOptions.go
Outdated
| opt.AddConstraint(defaultJointConstraint, NewJointConstraint(math.Inf(1))) | ||
| opt.metric = NewSquaredNormMetric() | ||
| opt.pathDist = NewZeroMetric() // By default, the distance to the valid manifold is zero, unless constraints say otherwise | ||
| opt.GoalArcScore = JointMetric |
There was a problem hiding this comment.
[q, nit] Was this meant to stay as GoalArcScore?
spieswl
suggested changes
Mar 27, 2023
raybjork
reviewed
Mar 28, 2023
…nstraint-system-in-anticipation-of-new-constraints-being-added
viambot
added
safe to test
raybjork
requested changes
Mar 28, 2023
raybjork
requested changes
Mar 28, 2023
There was a problem hiding this comment.
Ok I think I'm done with my first pass. Overall super excited about this. This is an awesome change!
At a high level the things that I am mainly concerned with surround the new ____Input types. I think we should figure out a good naming scheme for what we call Input StateInput SegmentInput because the word input is getting a little overloaded and now is a good time to establish how we want to talk about these going forward. Also, I don't see the need to add frames to these new structures. The frame can be a member on the constraintHandler, which should simplify things a bit.
| } | ||
|
|
||
| collisions := cg.collisions() | ||
| if len(collisions) > 0 { |
There was a problem hiding this comment.
We need to add this back now right?
There was a problem hiding this comment.
No, since we're no longer reporting distances here
There was a problem hiding this comment.
what is reportDistances doing as an argument if we are not doing this anymore?
There was a problem hiding this comment.
Once the PenetrationDepthMetric that I mentioned in my TODO is written, that will use true
viambot
added
safe to test
|
nfranczak
approved these changes
Mar 31, 2023
raybjork
requested changes
Apr 3, 2023
motionplan/metrics.go
Outdated
| return referenceframe.InputsL2Distance(segment.StartConfiguration, segment.EndConfiguration) | ||
| } | ||
|
|
||
| // TODO(pl): Writing a PenetrationDepthMetric will allow cbirrt to path along the sides of obstacles rather than terminating |
| } | ||
|
|
||
| collisions := cg.collisions() | ||
| if len(collisions) > 0 { |
There was a problem hiding this comment.
what is reportDistances doing as an argument if we are not doing this anymore?
viambot
added
safe to test
raybjork
approved these changes
Apr 3, 2023
viambot
added
safe to test
Availability
Quality
Performance
The above data was generated by running scenes defined in the
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR does the following:
spatialmath.Poseas a primary data structure for the internal specification of Goals in motion planning. Goals are now stored/passed internally mostly as StateMetrics, where e.g. a passed in Pose is used to create a distance function which will converge on that pose. This is already what was happening implicitly, but this change opens the door to planning in terms of robot configurations where a single spatialmath.Pose is not necessarily the appropriate way to specify a Goal (e.g. 2d planning)