Skip to content

Reduce pybind11 compilation memory part 3#382

Merged
ahcorde merged 9 commits intogazebosim:ign-math6from
scpeters:more_pybind11_translation_units
Mar 7, 2022
Merged

Reduce pybind11 compilation memory part 3#382
ahcorde merged 9 commits intogazebosim:ign-math6from
scpeters:more_pybind11_translation_units

Conversation

@scpeters
Copy link
Copy Markdown
Member

@scpeters scpeters commented Mar 6, 2022

🦟 Bug fix

Follow-up to #371 and #373.

Summary

This continues moving template instantiations from _ignition_math_pybind11.cc to a separate translation unit for templates in each of the following header files:

  • MassMatrix3.hh
  • Matrix3.hh
  • Matrix4.hh
  • Pose3.hh
  • Quaternion.hh
  • Triangle.hh
  • Triangle3.hh

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@github-actions github-actions bot added Gazebo 1️1️ Dependency of Gazebo classic version 11 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress 🏰 citadel Ignition Citadel labels Mar 6, 2022
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 6, 2022

Codecov Report

❗ No coverage uploaded for pull request base (ign-math6@6340f89). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head f39cfa0 differs from pull request most recent head 36e500a. Consider uploading reports for the commit 36e500a to get more accurate results

Impacted file tree graph

@@             Coverage Diff              @@
##             ign-math6     #382   +/-   ##
============================================
  Coverage             ?   99.65%           
============================================
  Files                ?       67           
  Lines                ?     6380           
  Branches             ?        0           
============================================
  Hits                 ?     6358           
  Misses               ?       22           
  Partials             ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6340f89...36e500a. Read the comment docs.

ahcorde
ahcorde previously requested changes Mar 7, 2022
Copy link
Copy Markdown
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

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

I added this line

find_package(Python3 REQUIRED COMPONENTS Interpreter Development)

before find pybind11 and Jammy CI is fixed

@ahcorde
Copy link
Copy Markdown
Contributor

ahcorde commented Mar 7, 2022

I added this line

find_package(Python3 REQUIRED COMPONENTS Interpreter Development)

before find pybind11 and Jammy CI is fixed

#384

@scpeters scpeters requested a review from ahcorde March 7, 2022 17:20
@scpeters scpeters dismissed ahcorde’s stale review March 7, 2022 17:40

jammy fix already merged

@ahcorde ahcorde merged commit 3b3e7ab into gazebosim:ign-math6 Mar 7, 2022
@scpeters scpeters deleted the more_pybind11_translation_units branch March 7, 2022 18:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏰 citadel Ignition Citadel 🏢 edifice Ignition Edifice 🏯 fortress Ignition Fortress Gazebo 1️1️ Dependency of Gazebo classic version 11

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants