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

Create single-file installers #161

Merged
merged 3 commits into from
May 18, 2022
Merged

Create single-file installers #161

merged 3 commits into from
May 18, 2022

Conversation

mattwthompson
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Apr 22, 2022

Codecov Report

Merging #161 (01f63fc) into main (cb5aa10) will decrease coverage by 0.89%.
The diff coverage is n/a.

@mattwthompson mattwthompson marked this pull request as draft April 25, 2022 12:06
@mattwthompson mattwthompson force-pushed the installer branch 2 times, most recently from 5db57a6 to c0ee846 Compare May 12, 2022 00:54
@mattwthompson
Copy link
Member Author

The last run successfully built an installer and installed against it, but I haven't yet got tests running and passing. I'm assuming no OpenEye license is needed to run tests.

@mattwthompson
Copy link
Member Author

I have it all the way to some tests passing, but some are still failing. Could you have a look at the failing tests and see what I'm missing @jthorton? I can add the OpenEye license in if it's required, but I don't know if that would cover all test failures.

The other option would be to not run tests based on the installer - I'd prefer keeping them in at all possible.

@jthorton
Copy link
Contributor

@mattwthompson Thanks for putting this together, and sure happy to help but not sure where I should be looking for tests? in the single file installer CI I see it failing before getting to the tests or do I need to download the installer and run the tests locally?

@mattwthompson
Copy link
Member Author

I screwed something up in the last commit, here are some logs from earlier showing failing tests: https://gist.github.com/mattwthompson/0903635d42866f6826069edcc18c69cd

@j-wags
Copy link
Member

j-wags commented May 13, 2022

Ah, one things that I'm seeing in my current testing as well is the "can't assign fractional bond orders" one - Users need EITHER OpenEye OR (AmberTools + RDKit) to run fragmentation. The bespokefit test env and conda package only install the openff-toolkit-base package, which doesn't come with AmberTools.

Since the vast majority of users won't have access to OpenEye we should make sure the SFIs bundle AmberTools.

Separately, I asked OE for permission to bundle their toolkits in our SFIs and they did not approve it. So let's also make sure not to bundle OETK in the SFIs.

@mattwthompson
Copy link
Member Author

Updated openforcefield/toolkit-installer-constructor@5e072df and restarting now

@mattwthompson mattwthompson force-pushed the installer branch 2 times, most recently from 8beca7a to 4d2b89e Compare May 13, 2022 17:18
@mattwthompson
Copy link
Member Author

Most recent failures - PDB loading, QCArchive offline https://gist.github.com/mattwthompson/f39e6bc4a89275a2f4339e6578c9460b

@mattwthompson
Copy link
Member Author

Some tests are failing because they require OpenEye behavior - I'll open a PR to skip these tests in that case.

@mattwthompson
Copy link
Member Author

The single-file installers are being built (with AmberTools installed - no OpenEye license or software packaged) and seem to not only exist but also pass (most) tests). Most of the failures seem to be a hard OpenEye dependency, which I attempted to fix in #170. There's one test failing in that PR but otherwise is good, I think.

@mattwthompson
Copy link
Member Author

Okay, #170 is merged in and hopefully the tests pass here now.

Fix branch switching

Debug

Retrigger after pushing to toolkit-installer-constructor@bespokefit

Fix some paths to build artifacts

Fix typo

Debug

Fix path, streamline workflow

Fix SMIRNOFF angle prior literal validator (#169)

Bump codecov/codecov-action from 2.1.0 to 3.1.0

Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 2.1.0 to 3.1.0.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/master/CHANGELOG.md)
- [Commits](codecov/codecov-action@v2.1.0...v3.1.0)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

Fix SMIRNOFF angle prior literal validator (#169)

Bump codecov/codecov-action from 2.1.0 to 3.1.0

Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 2.1.0 to 3.1.0.
- [Release notes](https://github.com/codecov/codecov-action/releases)
- [Changelog](https://github.com/codecov/codecov-action/blob/master/CHANGELOG.md)
- [Commits](codecov/codecov-action@v2.1.0...v3.1.0)

---
updated-dependencies:
- dependency-name: codecov/codecov-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>

Fix typo

Fix typos

Fix typo

Fix version parsing, run tests even if mismatch

Install optional/test dependencies

Clean up, split out tests

Tinker
@mattwthompson mattwthompson force-pushed the installer branch 2 times, most recently from 194babf to 3ee7344 Compare May 17, 2022 21:09
@mattwthompson
Copy link
Member Author

Tests are passing; I'll do some final touch-ups, merge, and upload the installers to the latest release. As this does not touch functionality of the software itself, I don't think waiting for reviews is warranted, though feedback is always the case on a non-draft PR.

Please note that these are XTB + AmberTools (+ RDKit) bundles; adding a Psi4 bundle to this array is a separate task and, as noted above, none will be made with OpenEye Toolkits bundled in. I've updated the file names to reflect this, using the pattern

name: ${{ matrix.os }}_py${{ matrix.python-version }}_xtb.sh

Copy link
Member

@j-wags j-wags left a comment

Choose a reason for hiding this comment

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

Fantastic. Thanks a million, @mattwthompson!! (I agree a review wasn't needed on this, I just happened to be checking my email when the notification came in)

.github/workflows/installer.yml Outdated Show resolved Hide resolved
@mattwthompson mattwthompson marked this pull request as ready for review May 17, 2022 22:06
@mattwthompson mattwthompson merged commit 47aca43 into main May 18, 2022
@mattwthompson mattwthompson deleted the installer branch May 18, 2022 04:05
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