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

Update docker file (for latest nsys) #500

Merged
merged 2 commits into from
Apr 27, 2021
Merged

Conversation

pgrete
Copy link
Collaborator

@pgrete pgrete commented Apr 26, 2021

PR Summary

The updated cuda version on the host seems to have resulted in the docker nsys stalling (with tests timing out after three hours of "finalizing").
This updates also the docker image to use Cuda 11.3 with the latest version of nsys.
For reference: We use nsys profile to ensure there are not unintended host to device or vice versa copies during execution.

PR Checklist

  • Changes are summarized in CHANGELOG.md

@pgrete pgrete force-pushed the pgrete/update-ci-cuda113 branch from ee4b12a to 785a575 Compare April 26, 2021 16:28
@pgrete
Copy link
Collaborator Author

pgrete commented Apr 26, 2021

I now also updated the thresholds for the Overlapping SpaceInstances so that they are less restrictive (but still check the SpaceInstances themselves work), see also #489

@pgrete
Copy link
Collaborator Author

pgrete commented Apr 26, 2021

Just for reference: I manually triggered the full test suite and it passed.

Copy link
Collaborator

@jlippuner jlippuner 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. One tiny comment.

@@ -622,8 +622,9 @@ struct SmallNLongTBufferPack {
template <typename TimeType>
static void test_time(const TimeType time_default, const TimeType time_spaces,
const int nspaces) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

So we don't need nspaces anymore? In that case we should do:

Suggested change
const int nspaces) {
const int /*nspaces*/) {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will handle that in a separate PR not not trigger CI again.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Curious, is this because of -Wunused-parameter ? Why not remove the const int too?

@pgrete pgrete merged commit f11dedb into develop Apr 27, 2021
@pgrete pgrete deleted the pgrete/update-ci-cuda113 branch April 27, 2021 15:34
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.

4 participants