[Jenkinsfile] Improvements and simplifications #806

atamazov · 2021-03-19T15:46:04Z

[coverage] Use installable builds in 4 Full stages out of 9 (BF16/FP16/FP32, OCL/HIP, MI100/Vega)
[improvement] Get rid of MIOPEN_GPU_SYNC
[code quality] Remove useless cargo cult docker image name handling (resolves issue [Jenkinsfile][NFC] Remove docker image name handling -- it is useless and just clutters the file. #801)
[UX] Always show datatype in stage names. Remove useless comments and empty lines.
[UX] Remove 'Release' from stage names. Rename 'Fast' and 'Aux' stages to 'Smoke'.
[code quality] Use only nodes marked with 'nogpu' for tests that require no GPU.
[code quality] Require 'rocmtest' and 'miopen' labels on all nodes, including gfx908.

[Side note] All relevant CI nodes updated with 'nogpu', 'miopen and 'rocmtest' labels:

'nogpu' is set on nodes that are allowed to run tests that do not require a GPU. Currently, these are all nodes except MI100.
'rocmtest' means that the node belongs to the "ROCmSoftwarePlatform" github organization (which now includes MIOpen, AMDMIGraphX, MIOpenGEMM, MIOpenTensile).
'miopen' means that the node is allowed to run MIOpen tests.

…/HIP, MI100/Vega)

This reverts commit fe2558d.

# RESOLVED Conflicts: # Jenkinsfile

…ments and empty lines.

…GPU. Require 'rocmtest' for all nodes, including gfx908.

atamazov · 2021-03-19T23:56:54Z

@pfultz2 This Codecov works like a random number generator ;)

atamazov · 2021-03-20T14:21:43Z

@ce1adon @pfultz2 @shurale-nkn @JehandadKhan All CI tests passed. None substantial performance gain or drop observed. Please approve.

pfultz2 · 2021-03-20T19:04:37Z

Jenkinsfile

-
-def default_image_name() {
-    return 'miopen-v2'
+    return 'rocmtest && miopen && ' + name


This should be (rocmtest || miopen) so you can allocate nodes just for miopen in the future.

The idea is to allocate nodes exclusively for miopen by removing other labels (like migraphx) from the nodes. From the PR description:

[Side note] All relevant CI nodes updated with 'nogpu', 'miopen and 'rocmtest' labels:

'nogpu' is set on nodes that are allowed to run tests that do not require a GPU. Currently, these are all nodes except MI100.

'rocmtest' means that the node belongs to the "ROCmSoftwarePlatform" github organization (which now includes MIOpen, AMDMIGraphX, MIOpenGEMM, MIOpenTensile).

'miopen' means that the node is allowed to run MIOpen tests.

MIGraphX uses rocmtest || migraphx so you would need to remove both migraphx label and the rocmtest label.

Thanks! Currently I do not plan to be greedy and allocate nodes for MIOpen exclusively.

However -- I would appreciate if you find the hierarchic approach in this file more logical and borrow it for MIGraphX.

Jenkinsfile

pfultz2 · 2021-03-20T19:09:42Z

Jenkinsfile

+    if(flags.contains('-DBUILD_DEV=Off'))
+    {
+    	config_targets = 'install ' + config_targets
+    	flags = '-DCMAKE_INSTALL_PREFIX=../install ' + flags


Why aren't we using the default install prefix? The packages we build should be able to be used by others.

I tried once -- it didn't work and I do not know why. I can try once more , if you wish.

Ah yea it may not work because it may need to be sudo. Why are we adding an install step here? When we call make package it will run install into the build directory, but the package will use the correct install prefix.

I would like to test "installed" MIOpen. Some aspects of its behavior differ for DEV builds, e.g. binary cache is enabled. This may accelerate the tests. And recently I fiddled around with an issue which is not reproducible with DEV builds (#798).

Or you mean that install step is not necessary to test non-DEV build? IIRC at least installation of system databases is required.

Jenkinsfile

pfultz2 · 2021-03-20T19:21:28Z

This Codecov works like a random number generator ;)

Its not random. This is comparing against 7bd5999 which is not the latest on develop. The code coverage changes are from 1c72099.

It seems like Codecov is not finding the base commit correctly, I am not sure why as I dont see this problem with migraphx.

atamazov · 2021-03-20T21:01:26Z

@pfultz2 pls let me know if this can be merged (or we can discuss more..)

pfultz2 · 2021-03-21T22:50:38Z

pls let me know if this can be merged (or we can discuss more..)

This shouldn't have been merged yet until the broken packages was fixed.

atamazov · 2021-03-21T23:18:22Z

@pfultz2

This shouldn't have been merged yet until the broken packages was fixed.

Sorry, but I do not understand. Could you please explain a bit more?

atamazov added 14 commits March 17, 2021 21:29

Removed "Subset"

6db2226

Use installable builds in 4 Full stages out of 9 (BF16/FP16/FP32, OCL…

9c0d900

…/HIP, MI100/Vega)

Fix syntax error

59fea35

[tmp] Remove unneeded stages

fe2558d

Create install dir

ec5f440

Revert "[tmp] Remove unneeded stages"

355c489

This reverts commit fe2558d.

Merge branch 'develop' into ci-use-installable-builds

52b1882

# RESOLVED Conflicts: # Jenkinsfile

[Jenkinsfile] Get rid of MIOPEN_GPU_SYNC

c174b68

[Jenkinsfile] Remove docker image name handling (resolves #801)

fabbd88

[Jenkinsfile] Always show datatype in stage names. Remove useless com…

cbd5722

…ments and empty lines.

[Jenkinsfile] Remove 'Release' from stage names

11a8f2e

[Jenkinsfile] Rename 'Fast' and 'Aux' stages to 'Smoke'.

6693c14

[Jenkinsfile] Use nodes maked with 'nogpu' for tests that require no …

17c3fd1

…GPU. Require 'rocmtest' for all nodes, including gfx908.

[Jenkinsfile] Use only nodes with 'miopen' label

ce698e1

atamazov added testing urgency_high labels Mar 19, 2021

atamazov requested review from pfultz2, JehandadKhan, ce1adon and shurale-nkn March 19, 2021 15:46

This comment has been minimized.

Sign in to view

atamazov added the TESTING_CI_PASSED label Mar 20, 2021

JehandadKhan approved these changes Mar 20, 2021

View reviewed changes

pfultz2 reviewed Mar 20, 2021

View reviewed changes

Jenkinsfile Show resolved Hide resolved

pfultz2 reviewed Mar 20, 2021

View reviewed changes

Jenkinsfile Show resolved Hide resolved

atamazov merged commit 7ea7f9f into develop Mar 20, 2021

carlushuang mentioned this pull request Mar 21, 2021

[NHWC] support nhwc in ctest #765

Merged

This was referenced Mar 23, 2021

MIOpenTensile Part2 : Support low precision types #556

Merged

[Jenkinsfile] Fix & refactor build/install directory handling. Rework test for BUILD_DEV. #814

Merged

atamazov deleted the ci-use-installable-builds branch April 19, 2021 22:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Jenkinsfile] Improvements and simplifications #806

[Jenkinsfile] Improvements and simplifications #806

atamazov commented Mar 19, 2021 •

edited

Loading

This comment has been minimized.

atamazov commented Mar 19, 2021

atamazov commented Mar 20, 2021

pfultz2 Mar 20, 2021

atamazov Mar 20, 2021

pfultz2 Mar 21, 2021

atamazov Mar 21, 2021

pfultz2 Mar 20, 2021

atamazov Mar 20, 2021

pfultz2 Mar 21, 2021

atamazov Mar 21, 2021

pfultz2 commented Mar 20, 2021

atamazov commented Mar 20, 2021

pfultz2 commented Mar 21, 2021

atamazov commented Mar 21, 2021

[Jenkinsfile] Improvements and simplifications #806

[Jenkinsfile] Improvements and simplifications #806

Conversation

atamazov commented Mar 19, 2021 • edited Loading

This comment has been minimized.

atamazov commented Mar 19, 2021

atamazov commented Mar 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pfultz2 commented Mar 20, 2021

atamazov commented Mar 20, 2021

pfultz2 commented Mar 21, 2021

atamazov commented Mar 21, 2021

atamazov commented Mar 19, 2021 •

edited

Loading