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

Enable whole testsuite #1

Merged
merged 46 commits into from
Nov 20, 2024
Merged

Enable whole testsuite #1

merged 46 commits into from
Nov 20, 2024

Conversation

mcocdawc
Copy link
Contributor

@mcocdawc mcocdawc commented Nov 19, 2024

  • Enable automatic installation of dependencies, i.e. libdmet.

  • Enable nearly the whole test suite for GitHub Actions; i.e. I removed most conditional executions based on GITHUB_ACTIONS. Replaced the check for the GITHUB_ACTIONS environment variable with a check for QUEMB_SKIP_EXPENSIVE_TESTS, after all sometimes we also want to skip expensive tests in another environment.

  • Moved both packages into subdirectories of a src/ directory.
    This way we are forced to actually install for the test-suite and don't miss it anymore, if the automatic build-process gets broken. (As it was the case because of double python_requires arguments)

The GitHub actions are prepared to enable ruff linting and code analysis as part of the test suite:

    - name: Check formatting
      run: |
        ruff format --diff || true    # for the moment we want to always report success


    - name: Static analyis
      run: |
        ruff check . || true    # for the moment we want to always report success

As soon as the || true is removed we will have linting enforced and conditionally execute the test-suite only if static analysis actually succeeded.

similar to oimeitei/quemb#53

- introduce QUEMB_SKIP_EXPENSIVE_TESTS environment variable.
    sometimes we want to skip expensive tests even outside GitHub
    Actions
one task in #54
the command was `ruff check --fix .`
Only safe fixes where applied https://docs.astral.sh/ruff/linter/#fix-safety
now DMRG and UHF should fail
no BE1 for density matching possible
both `fragment_mo` and `embedding` fail
currently fails because of scratch dir mess
(And it actually works) :-D
set OMP_NUM_THREAD=1 makes the MO-coefficients deterministic
pyscf/pyscf#2243

thanks Minsik :-)
Due to
gkclab/libdmet_preview#20
gkclab/libdmet_preview#21
we can finally use again the canonical libdmet
@mcocdawc mcocdawc requested a review from mscho527 November 19, 2024 22:19
@mcocdawc mcocdawc requested a review from lweisburn November 19, 2024 22:19
Apparently the MO coefficients still differ, while the energy is spot
on. It most likely results from near-degeneracies and free rotations
between occupied orbitals. the test probably requires to supply MO
coefficients
Copy link
Member

Choose a reason for hiding this comment

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

iirc, readthedocs.io (at least the way it is configured now) pulls only from this file. since the website isn't pulling from this repo for now, it should be fine, but we should keep this in mind.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, then I would still rather use the setup.py as reference and not redundantly define dependencies. (if possible)

Copy link
Member

@mscho527 mscho527 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 to me!

@mcocdawc mcocdawc force-pushed the enable_whole_testsuite branch from 6252fde to 8316165 Compare November 20, 2024 18:57
ruff format --diff || true # for the moment we want to report always report success


- name: Static analyis
Copy link
Member

Choose a reason for hiding this comment

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

tiny tiny typo

Copy link
Member

Choose a reason for hiding this comment

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

otherwise, the changes look good to me! @mcocdawc

thanks Minsik :-)
@mcocdawc mcocdawc merged commit 14eb856 into main Nov 20, 2024
3 checks passed
@mscho527 mscho527 deleted the enable_whole_testsuite branch December 3, 2024 02:18
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