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

2045 Switch default builds to C++17 and remove old CUDA build #2047

Merged

Conversation

lifflander
Copy link
Collaborator

Fixes #2045

@codecov
Copy link

codecov bot commented Dec 20, 2022

Codecov Report

Merging #2047 (371c391) into develop (2b66dc0) will decrease coverage by 0.03%.
The diff coverage is n/a.

❗ Current head 371c391 differs from pull request most recent head 2490172. Consider uploading reports for the commit 2490172 to get more accurate results

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2047      +/-   ##
===========================================
- Coverage    84.45%   84.43%   -0.03%     
===========================================
  Files          730      729       -1     
  Lines        25842    25832      -10     
===========================================
- Hits         21825    21811      -14     
- Misses        4017     4021       +4     
Impacted Files Coverage Δ
src/vt/utils/adt/union.h 77.35% <ø> (-1.15%) ⬇️
src/vt/rdma/group/rdma_group.cc 0.00% <0.00%> (-12.50%) ⬇️
src/vt/worker/worker_group_comm.cc 0.00% <0.00%> (-7.15%) ⬇️
src/vt/runtime/component/diagnostic_enum_format.cc 0.00% <0.00%> (-2.71%) ⬇️
src/vt/vrt/collection/balance/read_lb.h 76.66% <0.00%> (-1.46%) ⬇️
src/vt/trace/trace_user_event.cc 37.25% <0.00%> (-1.21%) ⬇️
src/vt/vrt/collection/manager.impl.h 95.49% <0.00%> (-0.99%) ⬇️
src/vt/messaging/async_op_wrapper.cc 90.00% <0.00%> (-0.91%) ⬇️
src/vt/trace/trace_log.h 59.09% <0.00%> (-0.49%) ⬇️
... and 38 more

@PhilMiller PhilMiller changed the title 2045 Swtich default builds to c++17 remove old cuda build 2045 Switch default builds to c++17 remove old cuda build Dec 20, 2022
@PhilMiller PhilMiller changed the title 2045 Switch default builds to c++17 remove old cuda build 2045 Switch default builds to C++17 and remove old CUDA build Dec 20, 2022
@cz4rs
Copy link
Contributor

cz4rs commented Dec 20, 2022

A bunch of warnings promoted to errors in clang builds:

/vt/src/vt/utils/adt/union.h:727:5: error: expression result unused [-Werror,-Wunused-value]

@cz4rs
Copy link
Contributor

cz4rs commented Dec 20, 2022

Regarding the PR checks (PR description format) failure - possibly a regex didn't expect + character in the branch name?
Also note that the comment aggregating build results / errors is missing.

@thearusable
Copy link
Contributor

thearusable commented Dec 21, 2022

Regarding the PR checks (PR description format) failure - possibly a regex didn't expect + character in the branch name?
Also note that the comment aggregating build results / errors is missing.

Yes, regex is not expecting the + character: https://github.com/DARMA-tasking/check-pr-fixes-issue/blob/master/src/helpers.js#L3

@lifflander lifflander force-pushed the 2045-swtich-default-builds-to-c++17-remove-old-cuda-build branch from d21633d to 371c391 Compare January 3, 2023 23:33
@cz4rs
Copy link
Contributor

cz4rs commented Jan 9, 2023

icpc fails because of:

The following tests FAILED:
	399 - vt:TestPreconfig.test_vt_assert_no_mpi (SEGFAULT)

This might be worth investigating, but on the other hand the log is also littered with:

icpc: remark #10441: The Intel(R) C++ Compiler Classic (ICC) is deprecated and will be removed from product release in the second half of 2023. The Intel(R) oneAPI DPC++/C++ Compiler (ICX) is the recommended compiler moving forward. Please transition to use this compiler. Use '\''-diag-disable=10441'\'' to disable this message.

Can we drop icpc support right now?

@cz4rs
Copy link
Contributor

cz4rs commented Jan 9, 2023

On the other hand, clang-5.0 failure seems to be directly related to this PR:

In file included from /vt/src/vt/worker/worker_group_comm.cc:45:
/vt/src/vt/context/context.h:225:3: error: non-type template argument refers to subobject '&tls_static_str_thisWorker__'
  DeclareClassInsideInitTLS(Context, WorkerIDType, thisWorker_, no_worker_id)
  ^

@PhilMiller
Copy link
Member

For icpc, we'll probably want to pin whatever version of Intel's oneAPI is being supported by Trilinos and used by the Sandia apps.

The clang 5 failure in worker will go away when the removal PR is merged

@lifflander lifflander force-pushed the 2045-swtich-default-builds-to-c++17-remove-old-cuda-build branch from 371c391 to 2490172 Compare January 17, 2023 18:32
@lifflander lifflander merged commit 2728c72 into develop Jan 18, 2023
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.

Switch default builds to C++17, remove old CUDA build
4 participants