Skip to content

remove static designation on CEV broken functions - #3727

Merged
ethanglaser merged 3 commits into
uxlfoundation:mainfrom
ethanglaser:dev/eglaser-cev-unused-func-template
Aug 3, 2026
Merged

remove static designation on CEV broken functions#3727
ethanglaser merged 3 commits into
uxlfoundation:mainfrom
ethanglaser:dev/eglaser-cev-unused-func-template

Conversation

@ethanglaser

@ethanglaser ethanglaser commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

Resolves new failure in windows build for CEV (http://intel-ci.intel.com/f18af139-44fd-f1df-b2e8-d4f5ef20c6a0)
Run on this branch: http://intel-ci.intel.com/f18c4750-506a-f12c-874f-d4f5ef20c6a0


Checklist:

Completeness and readability

  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation to reflect the changes or created a separate PR with updates and provided its number in the description, if necessary.
  • Git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details).
  • I have resolved any merge conflicts that might occur with the base branch.

Testing

  • I have run it locally and tested the changes extensively.
  • All CI jobs are green or I have provided justification why they aren't.
  • I have extended testing suite if new functionality was introduced in this PR.

Performance

  • I have measured performance for affected algorithms using scikit-learn_bench and provided at least a summary table with measured data, if performance change is expected.
  • I have provided justification why performance and/or quality metrics have changed or why changes are not expected.
  • I have extended the benchmarking suite and provided a corresponding scikit-learn_bench PR if new measurable functionality was introduced in this PR.

@ethanglaser

Copy link
Copy Markdown
Contributor Author

/intelci: run

@david-cortes-intel

Copy link
Copy Markdown
Contributor

@Vika-F Would this bring these functions into the ABI checks?

@Vika-F

Vika-F commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

@Vika-F Would this bring these functions into the ABI checks?

@david-cortes-intel No, those changes should not affect the ABI as those functions are defined in cpp/daal/src (which contents are considered to have hidden visibility by default), and are not exported with DAAL_EXPORT.

#pragma clang diagnostic ignored "-Wunused-local-typedef"
#pragma clang diagnostic ignored "-Wsign-compare"
#pragma clang diagnostic ignored "-Wkeyword-macro"
#pragma clang diagnostic ignored "-Wunused-template"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks like this warning should be addressed instead of being ignored.

CC @Alexandr-Solovev .

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the only way to address this issue is to create a ticket for DPL team, but it will likely block the current PR. @ethanglaser Could you please create a ticket for DPL team and and a remark here to remove such line when the task is done?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, this it outside of our library's scope otherwise I'd address it myself. Will post link to ticket once I create.

@david-cortes-intel

Copy link
Copy Markdown
Contributor

/intelci: run

@ethanglaser

Copy link
Copy Markdown
Contributor Author

/intelci: run

FYI latest CEV run is in PR description. Still some errors on sklearnex build side, oneDAL build errors are resolved.

@david-cortes-intel david-cortes-intel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

CI errors are in BasicStatistics, so probably not related to these changes.

@ethanglaser
ethanglaser marked this pull request as ready for review August 3, 2026 18:56
Copilot AI review requested due to automatic review settings August 3, 2026 18:56
@ethanglaser
ethanglaser merged commit 40220bd into uxlfoundation:main Aug 3, 2026
17 of 30 checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses a Windows build failure related to CEV by adjusting linkage on a couple of DAAL decision-tree helper templates and by silencing a Clang warning in the oneAPI DPC++ sort implementation.

Changes:

  • Suppress Clang -Wunused-template warnings around the DPL kernel template include in the DPC++ sort primitive.
  • Remove static (internal linkage) from two DAAL templated helper functions used in decision tree model code.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
cpp/oneapi/dal/backend/primitives/sort/sort_dpc.cpp Adds a Clang diagnostic suppression for -Wunused-template in the existing diagnostic push/pop block.
cpp/daal/src/algorithms/dtrees/dtrees_model_impl.h Removes static from addLeafNodeInternal template helper.
cpp/daal/src/algorithms/dtrees/dtrees_model_impl_common.h Removes static from traverseNodesBF template helper.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants