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

Fix CPU usage 100% #1

Merged
merged 1 commit into from
Dec 27, 2022
Merged

Fix CPU usage 100% #1

merged 1 commit into from
Dec 27, 2022

Conversation

yikben-wong-dx
Copy link

Description

  • use omp_set_num_threads to limit number of threads allowed to be used

Motivation and Context

  • Cloth simulation used 100% of CPU Power

How Has This Been Tested?

This test uses the E2E setup in caisson_ros_pkg. Please make sure you update the caisson_ros_pkg repo and Talos repo.

  1. Before the changes
# Command in server
docker-compose -f ~/caisson_ws/repos/caisson_ros_pkg/docker/system/tsukuba.yml pull
docker-compose -f ~/caisson_ws/repos/caisson_ros_pkg/docker/system/tsukuba.yml up -d
. ~/caisson_ws/repos/caisson_ros_pkg/docker/system/scripts/tsukuba_simulation.bash

# Press Enter for all the window in tmux
# Command in 6082 Simulator VNC
ros2 launch oliver_ros_interface techfarm_caisson_launch.py rviz:=False
# Another Terminal in server
htop

The htop should show all CPU at close to 100%.
Please Ctrl + C all the windows in tmux only.

Screen Shot 2022-12-27 at 12 58 48

  1. Apply the changes
# Terminal in server
docker exec -it system /bin/bash
cd /home/docker/base
rm -rf CSF
git clone -b fix/all-cpu-100% [email protected]:DeepX-inc/CSF.git
mkdir -p CSF/build && cd CSF/build \
  && cmake -DBUILD_SHARED_LIBS=ON .. && make -j$(nproc) && make install -j$(nproc) && make clean

# Close terminal
  1. After the changes
  • Please go to the tmux terminal and use the all 4 previous commands in windows to start the system.
  • The htop should show normal CPU usage.
  • Use GeoViz and Simulation to check whether the height map is working.

Screen Shot 2022-12-27 at 12 59 29

Checklist

  • Have you filled the sections above?
  • Have you checked to ensure there aren't other open Pull Requests for the same GitHub Issues/ClickUp Task(s)?
  • Have you documented the changes in GitHub Issues and/or ClickUp Task(s) related to this Pull Request?
  • Have you added labels, where appropriate, to this Pull Request?

@yikben-wong-dx yikben-wong-dx added the bugfix Fixes a bug label Dec 27, 2022
@yikben-wong-dx yikben-wong-dx self-assigned this Dec 27, 2022
@yikben-wong-dx
Copy link
Author

The number of thread (6) does not contain any meaning. Please feel free to change it.

@matias-pavez-dx
Copy link

matias-pavez-dx commented Dec 27, 2022

I think it is okay as long as the heightmap or acceptance test results are not affected.

I guess this is for the perception team to decide.

@Apurrvv
Copy link

Apurrvv commented Dec 27, 2022

Yes, 6 is okay. I don't see a way to make this an argument but we one perception side will make a note of the value.

We need to make a change here also - https://github.com/DeepX-inc/caisson_ros_pkg/blob/43b38b90c8577788d07c38c77ef974a4980a6d35/docker/system/base.Dockerfile#L72

I will create a PR.

Copy link

@Apurrvv Apurrvv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, We will make a note. NUM_THREAD is set to 6 in CSF.

@yikben-wong-dx yikben-wong-dx merged commit a739c93 into master Dec 27, 2022
@yikben-wong-dx yikben-wong-dx deleted the fix/all-cpu-100% branch December 27, 2022 04:33
@matias-pavez-dx
Copy link

@Apurrvv

Yes, 6 is okay. I don't see a way to make this an argument but we one perception side will make a note of the value.

These are some ways to set the argument from outside:

  • Modify the library to allow such configuration (e.g. through function setter, constructor, ...).
  • Use compile options..

Anyways, this does not seem like an urgent concern.

@Apurrvv
Copy link

Apurrvv commented Dec 27, 2022

@Apurrvv

Yes, 6 is okay. I don't see a way to make this an argument but we one perception side will make a note of the value.

These are some ways to set the argument from outside:

  • Modify the library to allow such configuration (e.g. through function setter, constructor, ...).
  • Use compile options..

Anyways, this does not seem like an urgent concern.

I see, Got it. I will create an issue for now and create a PR in the future.
I don't see an option for creating an issue is it disabled by any chance?

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

Successfully merging this pull request may close these issues.

3 participants