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

tweak Amber(Tools) easyblock to run tests from top-level directory #2781

Merged

Conversation

akesandgren
Copy link
Contributor

(created using eb --new-pr)

@akesandgren
Copy link
Contributor Author

Don't merge yet. Looks like we need some patches in AT 21 and older to be able to do this.

@boegel
Copy link
Member

boegel commented Aug 29, 2022

@akesandgren Can you explain these changes a bit more? Why the change to .serial? Will this also work for older Amber(Tools) versions?

@boegel boegel changed the title run amber(tools) tests from the top dir tweak Amber(Tools) easyblock to run tests from top-level directory Aug 29, 2022
@akesandgren
Copy link
Contributor Author

That code path is only used for AmberTools 20 (and thus Amber 20) and higher. Running the tests from the top dir is the correct way to run tests according to the developers.
Instead of just running "make test" which just runs "make test.$(INSTALLTYPE)" we now explicitly run the serial, parallel, cuda_serial and cuda_parallel test suites according to what we're acutally building.

Thus this change makes the tests not run any pmemd tests when just building AmberTools whivch was the problem before.

(There are still tests in the suites that fail, but I'm chasing the developers to get better (less exact) testing of those reesults)

One still have to check the test logs for details but at least we get a working test log.

@akesandgren
Copy link
Contributor Author

Should be good to merge now in my opinion, updated PR's for relevant AmberTools versions submitted. Current Amber easyconfigs not affected since they aren't currently running the tests. (Except one that will need the top level Makefile patch to not include config.h)

@boegel
Copy link
Member

boegel commented Aug 29, 2022

Test report by @boegel

Overview of tested easyconfigs (in order)

  • SUCCESS AmberTools-21-foss-2021a.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
node3127.skitty.os - Linux RHEL 8.4, x86_64, Intel(R) Xeon(R) Gold 6140 CPU @ 2.30GHz (skylake_avx512), Python 3.6.8
See https://gist.github.com/5e1a152585ed95dd817aec886b51940a for a full test report.

Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants