Skip to content

Parallel II Estimator#1842

Open
BDoignies wants to merge 14 commits intomainfrom
parallelII
Open

Parallel II Estimator#1842
BDoignies wants to merge 14 commits intomainfrom
parallelII

Conversation

@BDoignies
Copy link
Copy Markdown
Contributor

PR Description

A new WIP branch for parallel Integral Invariant estimators.

Checklist

  • I have installed and run the pre-commit hooks (cf CONTRIBUTING.md).
  • Unit-test of your feature with Catch.
  • Doxygen documentation of the code completed (classes, methods, types, members...)
  • Documentation module page added or updated.
  • New entry in the ChangeLog.md added.
  • No warning raised in Debug mode.
  • All continuous integration tests pass (Github Actions)

@BDoignies
Copy link
Copy Markdown
Contributor Author

@dcoeurjo My last commit features the ordered (yet parallel version) of the estimator. I realized that because we rely on the input range for surfel, we have no need to build locally the surface and we can simply extract the required surfels from the range.

Not to blow my own trumpet, but I think the final code looks pretty good :) (And in my computer, even with only 2000 surfels and 4 threads it does run faster than the sequential one)

@BDoignies BDoignies requested a review from dcoeurjo March 12, 2026 09:29
@dcoeurjo
Copy link
Copy Markdown
Member

I will have a look, thanks.
In the meantime, could you please install the precommit scripts and run it on the edited files ?

Copy link
Copy Markdown
Member

@dcoeurjo dcoeurjo left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just the doc is missing and some LGPL headers. I've added another domain splitter and a unit test for that.
Code should be in "kernel/domains/" subfolder by the way

@@ -0,0 +1,150 @@
#include <ranges>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

add dgtal header

* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
**/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ownership missing ?
@file missing (cf doxygen error)

@@ -0,0 +1,135 @@
#pragma once

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

gel header here

dcoeurjo and others added 3 commits April 9, 2026 16:02
ZZMerge branch 'parallelII' of github.com:DGtal-team/DGtal into parallelII
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants