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

Add JET.jl runtest #61

Merged
merged 16 commits into from
Apr 8, 2024
Merged

Add JET.jl runtest #61

merged 16 commits into from
Apr 8, 2024

Conversation

albertomercurio
Copy link
Member

This feature adds JET.jl for type instabilities and bugs detection. For the moment it has detected several bugs, that have to be fixed in the future.

@albertomercurio albertomercurio marked this pull request as ready for review April 7, 2024 22:20
Copy link

codecov bot commented Apr 7, 2024

Codecov Report

Attention: Patch coverage is 95.52239% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 90.17%. Comparing base (999823f) to head (af9b237).

Files Patch % Lines
src/quantum_object.jl 97.77% 1 Missing ⚠️
src/quantum_operators.jl 85.71% 1 Missing ⚠️
src/time_evolution/mcsolve.jl 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #61      +/-   ##
==========================================
+ Coverage   90.07%   90.17%   +0.09%     
==========================================
  Files          18       18              
  Lines        1692     1699       +7     
==========================================
+ Hits         1524     1532       +8     
+ Misses        168      167       -1     

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

@ytdHuang
Copy link
Member

ytdHuang commented Apr 8, 2024

Shall we make code quality tests (including Aqua.jl and JET.jl) as an independent test GROUP instead of Core tests? Since this is just for checking the instability of the codes, not the core functionalities.

Let's say we call the new group GROUP="Code Quality", in the end of runtests.jl we can add:

if (GROUP == "All") || (GROUP == "Code Quality")
    include(joinpath(testdir, "aqua.jl"))
    include(joinpath(testdir, "JET.jl"))
end

and remove these two tests from core_tests list.

In this case, the CI pipeline can have an independent group which runs this Code Quality part of test with latest Julia version and Linux OS only. Furthermore, we can just set the [compat] of Aqua and JET for supporting only the latest version of Julia.

For the action setting file CI.yml, can add a section like what I did for the extensions in my package:

matrix:

  ... # the original settings for core functionalities

  include:
    - julia-version: '1'
      os: 'ubuntu-latest'
      arch: 'x64'
      group: 'Code Quality'

What do you think ?

@albertomercurio
Copy link
Member Author

Yes, good idea. I changed the files accordingly. Please check if everything is fine, I'm not an expert on CIs 😄

@albertomercurio
Copy link
Member Author

There are some fails on previous Julia versions. It doesn't seem to be related to this PR.

@albertomercurio
Copy link
Member Author

We can wait a while before they fix this issue.

@albertomercurio
Copy link
Member Author

Yes, good idea. I changed the files accordingly. Please check if everything is fine, I'm not an expert on CIs 😄

For the moment, every runtest imports JET, right? Is there a way to import it only for the Code QUality CIs?

Moreover, there is an error in the current CI you just changed.

@ytdHuang
Copy link
Member

ytdHuang commented Apr 8, 2024

Yes, good idea. I changed the files accordingly. Please check if everything is fine, I'm not an expert on CIs 😄

For the moment, every runtest imports JET, right? Is there a way to import it only for the Code QUality CIs?

Moreover, there is an error in the current CI you just changed.

Yeah, I was also trying to do this.

@ytdHuang
Copy link
Member

ytdHuang commented Apr 8, 2024

Yes, good idea. I changed the files accordingly. Please check if everything is fine, I'm not an expert on CIs 😄

For the moment, every runtest imports JET, right? Is there a way to import it only for the Code QUality CIs?

Moreover, there is an error in the current CI you just changed.

I think the error comes from the Manifest.toml. Although we didn't import JET for other versions of julia, but it's still in the [compat] list in Project.toml, and thus the pre-build process throws errors.

@albertomercurio
Copy link
Member Author

I think the error comes from the Manifest.toml. Although we didn't import JET for other versions of julia, but it's still in the [compat] list in Project.toml, and thus the pre-build process throws errors.

No now it is fine, you fixed the Code Quality CI error. The errors for previous Julia version doesn't seem to be related to this PR.

Anyway, I will probably make a dedicated environment for the code quality tests.

@ytdHuang
Copy link
Member

ytdHuang commented Apr 8, 2024

I think the error comes from the Manifest.toml. Although we didn't import JET for other versions of julia, but it's still in the [compat] list in Project.toml, and thus the pre-build process throws errors.

No now it is fine, you fixed the Code Quality CI error. The errors for previous Julia version doesn't seem to be related to this PR.

Anyway, I will probably make a dedicated environment for the code quality tests.

I mean, the compat errors between JET.jl and previous julia versions.
Maybe can take this issue as a reference

@albertomercurio
Copy link
Member Author

There is a compilation error related to only the 1.9 version. It doesn't seem related to this PR, since the error appears even for the main branch.

Anyway, I made a separate environment for the code quality tests. What do you think?

@ytdHuang
Copy link
Member

ytdHuang commented Apr 8, 2024

There is a compilation error related to only the 1.9 version. It doesn't seem related to this PR, since the error appears even for the main branch.

Anyway, I made a separate environment for the code quality tests. What do you think?

The error for Julia 1.9 relates to NonLinearSolve (SciML/NonlinearSolve.jl#402), we can bypass this one (should be automatically solved after they fixed it)

@ytdHuang ytdHuang merged commit 131f81c into main Apr 8, 2024
10 of 13 checks passed
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.

2 participants