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: add DynamicExpressions integration tests #1675

Merged
merged 5 commits into from
Jul 24, 2024

Conversation

MilesCranmer
Copy link
Contributor

DynamicExpressions.jl seems to be good at flagging issues in Enzyme, such as the below, so this PR therefore adds a simple integration test for it so that these issues will get flagged immediately.

The CI also pins DynamicExpressions to a specific version where it has been known to work, so any failures would indicate some kind of internal regression. Also, this is a separate CI run so won't be recorded in the main test suite.

This is also important for the stability of the PySR/SymbolicRegression.jl ecosystem which have a large number of non-technical users; I would want to prevent breakages for them when I eventually make Enzyme part of the default search code.

I restructured the CI so that packages can add their own integration tests fairly easily:

    matrix:
      version:
        - '1'
      os:
        - ubuntu-latest
      test:
        - DynamicExpressions
    steps:
      - uses: actions/checkout@v4
      - uses: julia-actions/setup-julia@v1
        with:
          version: ${{ matrix.version }}
      - uses: julia-actions/cache@v1
      - uses: julia-actions/julia-buildpkg@v1
      - name: "Run tests"
        run: |
            julia --color=yes --project=. -e 'using Pkg; pkg"instantiate"'
            julia --color=yes --project=test/integration -e 'using Pkg; pkg"dev ."; pkg"instantiate"'
            julia --color=yes --project=test/integration --threads=auto --check-bounds=yes -e test/integration/${{ matrix.test }}.jl
        shell: bash

All they need to do is create a new test/integration/{PackageName}.jl file and add it to the list.

@gdalle maybe you could rebase #1563 on this so we can have them in the same format?

.github/workflows/CI.yml Outdated Show resolved Hide resolved
@wsmoses
Copy link
Member

wsmoses commented Jul 24, 2024

SGTM!

@MilesCranmer
Copy link
Contributor Author

Cool, seems to work!

(I think the #1674 is a different issue)

@codecov-commenter
Copy link

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.35%. Comparing base (5fc2309) to head (9fcbcae).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #1675       +/-   ##
===========================================
+ Coverage   70.60%   96.35%   +25.74%     
===========================================
  Files          31        9       -22     
  Lines       12926      411    -12515     
===========================================
- Hits         9127      396     -8731     
+ Misses       3799       15     -3784     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wsmoses wsmoses merged commit 4b464ac into EnzymeAD:main Jul 24, 2024
36 of 55 checks passed
@MilesCranmer MilesCranmer deleted the mc/integration-test-de branch July 24, 2024 23:38
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.

3 participants