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

CI fails if matrix is larger than 256 #391

Closed
edmundmiller opened this issue Mar 25, 2021 · 10 comments
Closed

CI fails if matrix is larger than 256 #391

edmundmiller opened this issue Mar 25, 2021 · 10 comments
Assignees
Labels
tests Related to automated tests

Comments

@edmundmiller
Copy link
Contributor

edmundmiller commented Mar 25, 2021

https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idstrategymatrix

@edmundmiller edmundmiller added the tests Related to automated tests label Mar 25, 2021
@edmundmiller edmundmiller self-assigned this Mar 25, 2021
@junaruga
Copy link

I am curious where is the repository using CI which the number of cases in the matrix is larger than 256?

@edmundmiller
Copy link
Contributor Author

@junaruga #389 See here

@junaruga
Copy link

junaruga commented Mar 25, 2021

@emiller88 thanks. I can see the pull-request's number of checks (cases) is 12, not larger (bigger) than 256. I can not find a number more than 256 there. What does the 256 actually mean?
https://github.com/nf-core/modules/pull/389/checks

@edmundmiller
Copy link
Contributor Author

image
Check out these action here. https://github.com/nf-core/modules/actions/runs/687601349

It's only showing 12 because it never starts the 256 because of the matrix limit. See the documentation here

@ewels
Copy link
Member

ewels commented Jun 15, 2021

This could be useful: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#example-including-additional-values-into-combinations

runs-on: ${{ matrix.os }}
strategy:
  matrix:
    os: [macos-latest, windows-latest, ubuntu-18.04]
    node: [8, 10, 12, 14]
    include:
      # includes a new variable of npm with a value of 6
      # for the matrix leg matching the os and version
      - os: windows-latest
        node: 8
        npm: 6

So basically you make a "thin" minimal matrix of combinations and then inject in one or two additional use cases. For example, you could run a matrix of tags with a single version of nextflow and just docker, then have a single tag / test running for different versions of Nextflow / Singularity / Conda.

I have written a workflow like this myself somewhere, but I have no idea where and I can't find it now. But the syntax is not too difficult.

@ewels
Copy link
Member

ewels commented Jun 15, 2021

Example from @maxulysse: https://github.com/nf-core/sarek/blob/68b9930a74962f3c42eee71f51e6dd2646269199/.github/workflows/ci.yml#L173-L189

  tools:
    env:
      NXF_ANSI_LOG: false
    runs-on: ubuntu-latest
    strategy:
      matrix:
        tool: [Haplotypecaller, Freebayes, Manta, mpileup, MSIsensor, Strelka, TIDDIT]
        intervals: [--no_intervals, '']
        exclude:
          - tool: Manta
            intervals: --no_intervals
          - tool: MSIsensor
            intervals: --no_intervals
          - tool: Strelka
            intervals: --no_intervals
          - tool: TIDDIT
            intervals: --no_intervals

@ewels
Copy link
Member

ewels commented Jun 16, 2021

Just rediscovered this issue too: nf-core/tools#1081 which links to the PR where I implemented this: nf-core/diaproteomics#131

@grst
Copy link
Member

grst commented Sep 24, 2021

This doesn't seem to be an issue anymore in #739.
image

Did GitHub lift the limit or did @emiller88 do some magic?

@grst
Copy link
Member

grst commented Sep 24, 2021

scratch that, it is still a problem and only linting runs.

@edmundmiller
Copy link
Contributor Author

Closing this for now, with nf-test and what not, we're not really bumping up against it.

@edmundmiller edmundmiller closed this as not planned Won't fix, can't repro, duplicate, stale Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Related to automated tests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants