search

ros-infrastructure / rep

ROS Enhancement Proposals
http://www.ros.org/reps/rep-0000.html
149 stars 136 forks source link

[REP-2014] Benchmarking performance in ROS 2 #364

Closed vmayoral closed 1 year ago

vmayoral commented 1 year ago

Signed-off-by: Víctor Mayoral Vilches v.mayoralv@gmail.com

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2022-9-15/27394/2

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/robotic-processing-unit-and-robotcore/27562/21

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/rep-2014-rfc-benchmarking-performance-in-ros-2/27770/1

vmayoral commented 1 year ago

@christophebedard, @iluetkeb, @anup-pem, @dejanpan, @kylemarcey (and surely many others), this work is heavily inspired by your prior work. As pointed out in here, I'd love to have your views on this and wouldn't mind co-authoring it together.

ZhenshengLee commented 1 year ago

Any consideration of reusing the performance_test from from apex.ai to this REP? This repo has been used by osrf-tsc-performance-report

Thanks.

vmayoral commented 1 year ago

Any consideration of reusing the performance_test from from apex.ai to this REP?

No. @ZhenshengLee refer to the text to see the rationale and the ros2_tracing papers cited. Note these two were considered in the prior art. The low-overhead for real-time tracing capabilities achievable with LTTng (through ros2_tracing) is miles apart. It's a framework specifically created for that. Also, though popular, the approaches you list don't fullfil our goals, nor can be used to instrument arbitrary pieces of code in a grey-boxed manner (performance tests are pre-defined).

The recommendations in here should be applicable across ROS stacks without the need of having to write new tests. An artificially create applications to benchmark performance. We should be able to make measurements of real running systems. Building upon ros2_tracing delivers this.

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/feedback-on-rep-2008-ros2-acceleration-kernels-with-build-integration/27872/1

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/nav2-discussion-metrics-framework-for-quantitative-evaluation-of-navigation-performance/28082/3

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/nav2-discussion-metrics-framework-for-quantitative-evaluation-of-navigation-performance/28082/12

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/rep-2014-rfc-benchmarking-performance-in-ros-2/27770/3

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/hardware-acceleration-wg-meeting-13/28273/1

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/rep-2014-rfc-benchmarking-performance-in-ros-2/27770/11

audrow commented 1 year ago

Here's some feedback from a discussion on this REP at Open Robotics:

Overall this seems like a high-quality and valuable REP. I think a little more iteration and it should be good to go.

If I misunderstood, missed, or butchered anything, please correct me @mjeronimo @wjwwood @sloretz, and FYI @clalancette.

ltbj commented 1 year ago

@vmayoral FYI I was able to catch up here, and I like the proposal. Thanks! I did not find anything non-trivial to be missing. IMO we could discuss this in the next TSC meeting.

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/robotperf-benchmarks-the-benchmarking-suite-to-evaluate-robotics-computing-performance-using-ros-2/28742/1

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-hardware-acceleration-working-group-2022-dissemination-report-and-feedback-request/29260/1

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/new-fast-dds-performance-testing/29539/2

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/survey-on-open-source-and-benchmarking-for-robotics/29821/2

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/survey-on-open-source-and-benchmarking-for-robotics/29821/4

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/medical-ros-how-do-i-get-involved/29891/9

vmayoral commented 1 year ago

There's been multiple reviews and iterations of this over many months. All in-code comments have been addressed. There's also been enough time for others making suggestions above to send coherent contributions.

Following from the discussion yesterday at the ROS 2 HAWG (minutes), this version is ready to move forward and get merged into the REP's repo. As with any REP, future changes can improve the document.

@gbiggs, who's in charge now of REPs, what's the new process of getting them accepted? Can we move forward with this?

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-benchmark-open-source-release/30753/2

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-05-18/31587/1

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-06-15/32038/1

SteveMacenski commented 1 year ago

First off, I'm sorry to be harsh, but I think you'd rather me be honest and concise than verbose but still communicating the same thoughts in 10x as many words of fluff.

The first 178 lines of the REP are just background context, which is genuinely useful, but doesn't actually culminate in any prescriptive testing / evaluation procedures or test-stand setup standardization. There is no standard within this proposed standard. The best I see are 2 non-tangible recommendations without any specificity.

The proposed REP points to a package, but a package is not a standard nor is it a proxy for a written set of instructions, explicit aims and processes, and requirements which might have been used to create a particular implementation. This is a real problem if you're proposing a standard to fit an implementation rather than writing an standard based on requirements and needs and then proposing an implementation alongside it that meets the standard.

Personally, I would not approve this since the document contains no standard.

Separately, I think more or less this sentence included in the REP explains why this probably shouldn't be an REP

There are no globally accepted industry standards for benchmarking robotic systems.

If there is not even generally understood standards or benchmarking outlines for robotics, it seems like we're just making Standard #14 rather than consolidating a generally recognized set of things that are useful in the application of robotics benchmarking to ROS. All other REPs are more or less the consolidation or proposition of best practices in robotics as ROS-specifics. This doesn't seem sufficiently mature to be admitted as an REP until there is a set of globally accepted standards which we're adapting for ROS uses.

This feels better suited as a paper or something in literature. I applaud the effort, something like this to standardize how we take performance metrics of ROS based systems has great utility, but I think for a standard it needs to have more detail wrt the process and fulfilled requirements & be based on agreed upon best practices

vmayoral commented 1 year ago

First off, I'm sorry to be harsh, but I think you'd rather me be honest and concise than verbose but still communicating the same thoughts in 10x as many words of fluff.

The first 178 lines of the REP are just background context, which is genuinely useful, but doesn't actually culminate in any prescriptive testing / evaluation procedures or test-stand setup standardization. There is no standard within this proposed standard. The best I see are 2 non-tangible recommendations without any specificity.

The proposed REP points to a package, but a package is not a standard nor is it a proxy for a written set of instructions, explicit aims and processes, and requirements which might have been used to create a particular implementation. This is a real problem if you're proposing a standard to fit an implementation rather than writing an standard based on requirements and needs and then proposing an implementation alongside it that meets the standard.

Personally, I would not approve this since the document contains no standard.

Thanks for the input @SteveMacenski. Processing this now. Let me take your feedback and try re-iterating the content and throw it back to you and others for review before the next meeting.

vmayoral commented 1 year ago

Separately, I think more or less this sentence included in the REP explains why this probably shouldn't be an REP

There are no globally accepted industry standards for benchmarking robotic systems.

If there is not even generally understood standards or benchmarking outlines for robotics, it seems like we're just making Standard #14 rather than consolidating a generally recognized set of things that are useful in the application of robotics benchmarking to ROS. All other REPs are more or less the consolidation or proposition of best practices in robotics as ROS-specifics. This doesn't seem sufficiently mature to be admitted as an REP until there is a set of globally accepted standards which we're adapting for ROS uses.

I disagree with this input @SteveMacenski.

As discussed during the last TSC meeting, it's relevant to understand that there're different types of REPs (see REP-0001). What you refer to is the Standards Track REPs. This is not one of them, and never meant to be. This has always been an Informational REP and content/structure follows various other existing/approved ones. Let me quote that for you:

An Informational REP describes a ROS design issue, or provides general guidelines or information to the ROS community, but does not propose a new feature. Informational REPs do not necessarily represent a ROS community consensus or recommendation, so users and implementors are free to ignore Informational REPs or follow their advice.

Having remarked all of this, as pointed out above, I think there's value in your input. Thanks for that. Let me see what I can do about it to improve this REP PR.

SteveMacenski commented 1 year ago

Ah I see, that is my mistake then. I was not aware of the different REP tracks :smile:

Given that context, I think that this is sufficient, but some additional high-level description would aid in the value to a reader to describe a bit about what ros2_tracing is measuring / doing, in particular. But, that's more of a request for some useful context more than the standards-level formality I was thinking previously.

I retract my previous hard-line position!

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-07-20/32525/1

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/project-proposal-ros2-tool-to-collect-topic-metrics/32790/2

kscottz commented 1 year ago

Hi Everyone,

This REP was reviewed and voted on by the ROS 2 TSC in our August 2023 meeting.. The vote was in favor of rejecting REP-2014. We plan to merge REP-2014 with status "rejected" so the information is still available.

The anonymous feedback from the TSC members was that the REP contains valuable information, but does not constitute a community standard. Some TSC members suggested REP-2014 would better serve the community as part of the ROS 2 Docs or as a stand-alone blog post.

clalancette commented 1 year ago

Based on the TSC decision, I've marked this REP as "Rejected", and will merge it as such.

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-08-17/33061/4

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-08-17/33061/6

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-08-17/33061/10

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-08-17/33061/11

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-08-17/33061/12

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-08-17/33061/16

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-08-17/33061/17

ros-discourse commented 1 year ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-08-17/33061/25

ros-discourse commented 8 months ago

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-hardware-acceleration-working-group-2023-summary-and-new-directions-for-2024/35454/1