conda-forge / ambertools-feedstock

A conda-smithy repository for ambertools.
BSD 3-Clause "New" or "Revised" License
8 stars 14 forks source link

Update to 23.0 #102

Closed jaimergp closed 1 year ago

jaimergp commented 1 year ago

Checklist

cc @dacase

conda-forge-webservices[bot] commented 1 year ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

Documentation on acceptable licenses can be found here.

jaimergp commented 1 year ago

@dacase, do you have a CHANGELOG or something like that to see which components have been removed?

ffgbsa is also missing according to the tests, so I want to see whether that's an error or just that this is not shipped anymore. Can you take a look at the test script?

dacase commented 1 year ago

ffgbsa has indeed been removed -- it was a part of something called AmberLite, which is gone. I don't have a CHANGELOG that indicates what has been added/removed.

conda-forge-webservices[bot] commented 1 year ago

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict. Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

conda-forge-webservices[bot] commented 1 year ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

Documentation on acceptable licenses can be found here.

dacase commented 1 year ago

A second release candidate is now ready: wget https://ambermd.org/downloads/AmberTools23_rc2.tar.bz2

This might be worth looking at, since it (among other things) fixes pytraj problems in OSX.

Also, there are two new executables that can be tested with "-h": edgembar -h and ndfes -h.

I'll try to see if I can get any useful info from the failing checks (above), and post any ideas here.

dacase commented 1 year ago

What would you think about removing the test on packmol-memgen -h? That fails for our builds as well, but we never test for it. If conda-forge requires the test, I can try to work on it, but would prefer to work on other issues.

dacase commented 1 year ago

Update: release candidate 3 is now available: https://ambermd.org/downloads/AmberTools23_rc3.tar.bz2 (same as before, except rc2 -> rc3

Aside: I'm not very experienced in reading azure pipeline logs. If someone could summarize, that would help. It looks like everything builds(?), but that the packmol-memgen -h execution is failing; this should be removed (see above). Are any other tests being run? What can I do to help get this closer to being ready to go? We may be ready on the Amber side by April 15, although there is flexibility there.

....thanks

dacase commented 1 year ago

Update: release candidate 4 is now available: https://ambermd.org/downloads/AmberTools23_rc4.tar.bz2 (same as before, except rc3 -> rc4

It would be great to get an update on what would be needed to get this out the door. We are almost ready to announce the release. Or, if this is too busy a time, is there some estimate of when the next steps might take place? (We could release a source-code version, and leave the conda update until later, if that is necessary.)

...thanks!

jaimergp commented 1 year ago

@dacase, the download links are giving me 404. Are those correct?

dacase commented 1 year ago

Sorry about that. RC4 was incorrectly built, and I've not yet created RC5. I'll ping you as soon as it is ready (should be later today).

dacase commented 1 year ago

OK: RC5 should be available: https://ambermd.org/downloads/AmberTools23_rc5.tar.bz2

conda-forge-webservices[bot] commented 1 year ago

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict. Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

conda-forge-webservices[bot] commented 1 year ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

Documentation on acceptable licenses can be found here.

jaimergp commented 1 year ago

@dacase, there are some test errors:

2675 file comparisons passed
30 file comparisons failed (2 of which can be ignored)
8 tests experienced errors
dacase commented 1 year ago

The real errors are actually few (but more than we see with conventional builds on Linux.)

  1. Do you have access to the .dif files from the test suite? Or is there some way to run the test suite (or parts of it) "by hand"? (see below)
  2. The linux_64_numpy1.21python3.10.____cpython error happens very early on, and looks like some incompatibility with conda's python3.10. Does condaforge allow us to not build for 3.10? (It looks like this version is skipped for OSX.) Or: do you see anything we might do to address this problem?

That said, I'm not quite sure how best to address the problems that are found. Most of the failures can be "fixed" by modifying the test suite -- these are very rare corner cases that might happen even with a conventional build. But about 25 of the failures are all of one feature that is actually used. So, can I either "build locally", or (better) download and install the conda package as it exists now, in order to see for myself what happens when I try to run the test suite?

...thx...dac

jaimergp commented 1 year ago

All build artifacts are here:

https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=690832&view=artifacts&pathAsName=false&type=publishedArtifacts

The right side of each row should have a three-dot menu with a download button. You need the conda_artifacts_* one. Inside, you should find a .conda archive you can install with conda.

Let me know if you need further instructions to get you started.

dacase commented 1 year ago

Ah...at least in my browser, the three-dot menu only appears if you hover the mouse over the line, and I had missed that option. I'll give this a try.

dacase commented 1 year ago

OK, I'm making progress. Good news is that the 25 errors mentioned above are all trivial. I'll have to poke around a bit more to see how to fix the tests. And I'm still not sure how to handle the python3.10 issue. I may be a day or two before I can make much progress here.

jaimergp commented 1 year ago

And I'm still not sure how to handle the python3.10 issue.

This is a metadata issue at the solver level, nothing to do with the source or Ambertools itself, no worries.

dacase commented 1 year ago

@jaimergp wrote: you should find a .conda archive you can install with conda.

I created a new conda envionment, followed by conda install ambertools-23.0rc5-py38h7e40819_0.conda. But this seems incomplete, especially in not having a python executable in the AMBERHOME/bin directory. [If I install AmberTools22 from the conda-forge channel, there is much more in the bin directory, including a python.] Do I need to do something else?

Related: can you say more about how pipelines are running the tests? The final conda package (correctly) doesn't have the test files, but I'm thinking you must have built with -DINSTALL_TESTS=TRUE at some point; or was something else done? I've been trying to hack a test run based on just what is in ambertools-23.0rc5-py38h7e40819_0.conda, but so far am getting stuck in finding the correct python executable to use.

Thanks for any pointers here.

jaimergp commented 1 year ago

Installing from an explicit local file will only unpack that archive. You then need to run conda update --all to "resolve" the environment with the dependencies. That should provide everything.

Alternatively, you can also run conda build --test path/to/ambertools.XYZ.conda, which should run the same pipelines as in CI.

jaimergp commented 1 year ago

Tests are built at test-time. See this block.

dacase commented 1 year ago

Thanks for the help. I've run the tests on linux (python 38). All the reported failures are trivial ones. (There is an optional comma in an output file that is different in the conda build from the test case targets; this leads to most of the errors. The conda build is getting parmed from conda-forge, which has a slightly more recent version than is in AmberTools, but that is fine. There are one or two insignificant round-off errors, of the sort we see with different compilers and Linux distros.)

So: is my say-so enough here? Could we plan to release once the final source tarfile is available (obviously with a final check like the one described above)? I expect the final release candidate (which I hope will turn in to the actual release) to be available in a day or two--certainly in time for our Zoom call on April 25. Target date for the release is end of the month.

Finally: can I (and other Amber developers) get access to exactly what users will actually install prior to the official release? My tests above were based on files in the conda_artifacts folder, but I'd like to make sure we have precise instructions for the actual files that will end up on conda-forge. And, I could use those to download and test things on OSX, and on variants of Linux.

jaimergp commented 1 year ago

The artifacts final users will get are essentially the same we get in CI. Once the final release is available, they should be equivalent and you can debug as much as needed.

I am fine with ignoring those trivial errors, thank you for looking into them! We'll disable the test suite before merging the final version.

The only question I have left: what ParmEd version are you vendoring this time? Do we need to bump the lower bound here? There was a question about the upper bound too in a separate PR (see https://github.com/conda-forge/ambertools-feedstock/pull/105). Would that be ok?

dacase commented 1 year ago

I am working with Jason Swails (author of parmed) to resolve the versioning question. Right now, it looks to me like typing parmed --version may not be giving the correct version number.

jaimergp commented 1 year ago

@conda-forge-admin, please rerender

conda-forge-webservices[bot] commented 1 year ago

Hi! This is the friendly automated conda-forge-linting service.

I was trying to look for recipes to lint for you, but it appears we have a merge conflict. Please try to merge or rebase with the base branch to resolve this conflict.

Please ping the 'conda-forge/core' team (using the @ notation in a comment) if you believe this is a bug.

conda-forge-webservices[bot] commented 1 year ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

Documentation on acceptable licenses can be found here.

jaimergp commented 1 year ago

Ok the py310 on linux-64 error is due to this PR needing some work:

Edit: should work now

conda-forge-webservices[bot] commented 1 year ago

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe) and found some lint.

Here's what I've got...

For recipe:

conda-forge-webservices[bot] commented 1 year ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

jaimergp commented 1 year ago

This is green now and we can think of merging to the rc branch. Let me know when you are ready.

Also I need written consent of all the new maintainers (a thumbs up is enough).

dacase commented 1 year ago

@jaimergp Actually, rc6 is actually the final product. So, if you wish, this could be published to conda-forge/ambertools. I'm happy with that, or with merging to the rc branch. If it's not publisehd to the main conda-forge channel, will there be a site (such as anaconda.org/jaimegrp/ambertools or similar) that I can use for a test install?

jaimergp commented 1 year ago

If merged to rc, you will do conda create -n ambertools-23-rc -c conda-forge/label/ambertools_rc -c conda-forge ambertools.

mikemhenry commented 1 year ago

Sweet, I will try making a PR off the RC that gets osx-arm64 working :mechanical_arm:

jaimergp commented 1 year ago

Ok, automerge is on. Once everything turns green, this will be merged to rc and available from conda-forge/label/ambertools_rc. 🤞

github-actions[bot] commented 1 year ago

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

Thus the PR was passing, but could not be merged (error={merge_status_message}).

mattwthompson commented 1 year ago

Error is weird but the bot probably didn't merge because this is still a draft?

https://conda-forge.org/docs/maintainer/infrastructure.html#automerge

mikemhenry commented 1 year ago

If anyone has the power to merge this in, that would be welcome!