build MPI - Githubissues

njzjz commented 9 months ago

Checklist

[x] Used a personal fork of the feedstock to propose changes
[x] Bumped the build number (if the version is unchanged)
[ ] Reset the build number to 0 (if the version changed)
[x] Re-rendered with the latest conda-smithy (Use the phrase code>@<space/conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
[x] Ensured the license file is being packaged.

Fix #132.

njzjz commented 9 months ago

@conda-forge-admin, please rerender

conda-forge-webservices[bot] commented 9 months 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:

The source section contained an unexpected subsection name. string is not a valid subsection name.

njzjz commented 9 months ago

@conda-forge-admin, please rerender

conda-forge-webservices[bot] commented 9 months 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.

njzjz commented 9 months ago

Got this error for osx-arm64:

2024-01-03T08:16:13.8739220Z + conda install --yes --no-deps --force-reinstall -p /Users/runner/miniforge3/conda-bld/ambertools_1704269182095/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol xorg-xproto xorg-libx11
2024-01-03T08:16:13.8769280Z + local cmd=install
2024-01-03T08:16:13.8842690Z + case "$cmd" in
2024-01-03T08:16:13.8871280Z + __conda_exe install --yes --no-deps --force-reinstall -p /Users/runner/miniforge3/conda-bld/ambertools_1704269182095/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol xorg-xproto xorg-libx11
2024-01-03T08:16:13.8873080Z + /Users/runner/miniforge3/bin/conda install --yes --no-deps --force-reinstall -p /Users/runner/miniforge3/conda-bld/ambertools_1704269182095/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehol xorg-xproto xorg-libx11
2024-01-03T08:16:16.5440100Z Channels:
2024-01-03T08:16:16.5440880Z  - conda-forge
2024-01-03T08:16:16.5441260Z Platform: osx-64
2024-01-03T08:16:50.9159540Z Collecting package metadata (repodata.json): ...working... done
2024-01-03T08:16:51.2541850Z Solving environment: ...working... failed
2024-01-03T08:16:51.2545380Z 
2024-01-03T08:16:51.2546940Z InvalidSpec: The package "conda-forge/osx-arm64::python_abi==3.9=4_cp39" is not available for the specified platform

It's not related to this PR but should be resolved.

@jaimergp, could you please clarify why they need to be reinstalled? Should they be installed for the host or target platform? Thanks!

njzjz commented 9 months ago

Oh, I see these comments:

https://github.com/conda-forge/ambertools-feedstock/blob/f14f8759cb85823b5bd1a62403e7167be1f4469f/recipe/build.sh#L39-L45

I think it should be reinstalled for the host platform.

njzjz commented 9 months ago

All builds pass. Now I'll build for other Python versions.

njzjz commented 9 months ago

@conda-forge-admin, please rerender

njzjz commented 8 months ago

Hi @conda-forge/ambertools, could someone take a review and merge the PR? Everything should be fine.

mattwthompson commented 6 months ago

I think this is good to go, I'm not sure if I'm missing something

dacase commented 6 months ago

I've not done a detailed review, but this looks fine to me, and will be of use to users.

But just a note: release candidates for AmberTools24 will be coming out starting next week. Conda-forge work should probably wait until the second or third of these -- I'll let people know when I think things are very close to final. So, one might consider waiting for AmberTools24 to actually push an MPI-enabled version to users. But I'm fine with doing it now as well.

mattwthompson commented 6 months ago

@dacase could you raise a new issue about version 24? It'll either get lost here or hijack this thread

xiki-tempula commented 6 months ago

I wonder if there is any update for this PR? This is a very valuable addition to the current conda package as it allows one easily deploy test the MPI version of sander in the unit tests. There are upside for merging this now would be that the ambertools 23 is quite stable now and we might want to inject this when every other part is stable instead of ambertools 24 when other part of the code might still need some testing.

xiki-tempula commented 6 months ago

@conda-forge-admin, please rerender

xiki-tempula commented 6 months ago

Might also be good to add a sander.MPI -h to https://github.com/conda-forge/ambertools-feedstock/blob/main/recipe/run_test.sh to check the binary runs.

mattwthompson commented 6 months ago

On osx-arm64 only:


  File "/Users/runner/miniforge3/lib/python3.10/site-packages/urllib3/response.py", line 761, in _error_catcher
    raise ProtocolError(arg, e) from e
urllib3.exceptions.ProtocolError: ('Connection broken: IncompleteRead(7551886 bytes read, 562277331 more expected)', IncompleteRead(7551886 bytes read, 562277331 more expected))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/runner/miniforge3/bin/conda-build", line 11, in <module>
    sys.exit(execute())
  File "/Users/runner/miniforge3/lib/python3.10/site-packages/conda_build/cli/main_build.py", line 581, in execute
    api.build(
  File "/Users/runner/miniforge3/lib/python3.10/site-packages/conda_build/api.py", line 250, in build
    return build_tree(
  File "/Users/runner/miniforge3/lib/python3.10/site-packages/conda_build/build.py", line 3762, in build_tree
    packages_from_this = build(
  File "/Users/runner/miniforge3/lib/python3.10/site-packages/conda_build/build.py", line 2547, in build
    try_download(m, no_download_source=False, raise_error=True)
  File "/Users/runner/miniforge3/lib/python3.10/site-packages/conda_build/render.py", line 757, in try_download
    source.provide(metadata)
  File "/Users/runner/miniforge3/lib/python3.10/site-packages/conda_build/source.py", line 1038, in provide
    unpack(
  File "/Users/runner/miniforge3/lib/python3.10/site-packages/conda_build/source.py", line 171, in unpack
    src_path, unhashed_fn = download_to_cache(
  File "/Users/runner/miniforge3/lib/python3.10/site-packages/conda_build/source.py", line 105, in download_to_cache
    download(url, path)
  File "/Users/runner/miniforge3/lib/python3.10/site-packages/conda/exports.py", line 408, in download
    return _download(url, dst_path, md5=md5sum, sha256=sha256, size=size)
  File "/Users/runner/miniforge3/lib/python3.10/site-packages/conda/common/io.py", line 85, in decorated
    return f(*args, **kwds)
  File "/Users/runner/miniforge3/lib/python3.10/site-packages/conda/gateways/connection/download.py", line 66, in download
    download_inner(
  File "/Users/runner/miniforge3/lib/python3.10/site-packages/conda/gateways/connection/download.py", line 122, in download_inner
    for chunk in resp.iter_content(chunk_size=CHUNK_SIZE):
  File "/Users/runner/miniforge3/lib/python3.10/site-packages/requests/models.py", line 818, in generate
    raise ChunkedEncodingError(e)
requests.exceptions.ChunkedEncodingError: ('Connection broken: IncompleteRead(7551886 bytes read, 562277331 more expected)', IncompleteRead(7551886 bytes read, 562277331 more expected))

mattwthompson commented 6 months ago

It looks like there's a lot in recipe/run_test.sh that could be cleaned up; the sander command isn't currently tested, MPI or not

njzjz commented 5 months ago

On osx-arm64 only:

This looks like a network issue.

Might also be good to add a sander.MPI -h to https://github.com/conda-forge/ambertools-feedstock/blob/main/recipe/run_test.sh to check the binary runs.

sander -h doesn't return a zero code. Perhaps we could test which sander and which sander.MPI.

sander -h; echo $?

     mdfil: Error unknown flag: -h                                                                                                                                                                                                                                                              

     usage: sander  [-O|A] -i mdin -o mdout -p prmtop -c inpcrd -r restrt
                   [-ref refc -x mdcrd -v mdvel -e mden -frc mdfrc -idip inpdip -rdip rstdip -mdip mddip 
                   -inf mdinfo -radii radii -y inptraj -amd amd.log -scaledMD scaledMD.log] -cph-data -ce-data <file>-host ipi_hostname -port ipi_port
Consult the manual for additional options.
1

mattwthompson commented 5 months ago

This looks like a network issue.

Yeah, somewhat suspicious that it's only on one architecture ... maybe the difference is when those runs started?

Perhaps we could test ...

Sorry to be unclear, that file could be improved by going through those commands and replacing them with something that runs like sander --version etc. I think which sander etc. would also be fine

I also don't think those changes are necessary to get this merged! They are "nice to have"s

mattwthompson commented 5 months ago

@conda-forge-admin, please restart ci

mattwthompson commented 5 months ago

The build seem (?) to be getting past the download step, so maybe they're going to succeed

xiki-tempula commented 5 months ago

Yes, sander --version seems to be a good test.

conda-forge-webservices[bot] commented 5 months 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:

This recipe is using a compiler, which now requires adding a build dependence on {{ stdlib("c") }} as well. For further details, please see https://github.com/conda-forge/conda-forge.github.io/issues/2102.

mattwthompson commented 5 months ago

This is probably fine but I want #137 and #139 merged upstream and back into this branch before merging this one

conda-forge-webservices[bot] commented 5 months 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:

Failed to even lint the recipe, probably because of a conda-smithy bug :cry:. This likely indicates a problem in your meta.yaml, though. To get a traceback to help figure out what's going on, install conda-smithy and run conda smithy recipe-lint . from the recipe directory.

conda-forge-webservices[bot] commented 5 months 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:

This recipe is using a compiler, which now requires adding a build dependence on {{ stdlib("c") }} as well. For further details, please see https://github.com/conda-forge/conda-forge.github.io/issues/2102.

conda-forge-webservices[bot] commented 5 months 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.

njzjz commented 5 months ago

@conda-forge-admin, please rerender

mattwthompson commented 5 months ago


+ sander --version
sander: Version 22.0
+ sander.MPI --version
/home/conda/feedstock_root/build_artifacts/ambertools_1714660402389/test_tmp/run_test.sh: line 50: sander.MPI: command not found
WARNING: Tests failed for ambertools-23.6-nompi_py312h5191d88_102.conda - moving package to /home/conda/feedstock_root/build_artifacts/broken
TESTS FAILED: ambertools-23.6-nompi_py312h5191d88_102.conda

##[error]Bash exited with code '1'.

njzjz commented 5 months ago



+ sander --version

sander: Version 22.0

+ sander.MPI --version

/home/conda/feedstock_root/build_artifacts/ambertools_1714660402389/test_tmp/run_test.sh: line 50: sander.MPI: command not found

WARNING: Tests failed for ambertools-23.6-nompi_py312h5191d88_102.conda - moving package to /home/conda/feedstock_root/build_artifacts/broken

TESTS FAILED: ambertools-23.6-nompi_py312h5191d88_102.conda

##[error]Bash exited with code '1'.

These are nompi builds...

mattwthompson commented 5 months ago

Haha, that makes more sense

njzjz commented 5 months ago

openmpi tests encounter the same error as https://github.com/conda-forge/openmpi-feedstock/issues/152

mattwthompson commented 5 months ago

That's weird. Any ideas besides bringing down openssh ... ?

njzjz commented 5 months ago

I set export OMPI_MCA_plm_ssh_agent=false as suggested by https://github.com/conda-forge/openmpi-feedstock/issues/152#issuecomment-2066413862.

njzjz commented 5 months ago

I set export OMPI_MCA_plm_ssh_agent=false as suggested by conda-forge/openmpi-feedstock#152 (comment).

Oh, it doesn't work :(

github-actions[bot] commented 5 months ago

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

I considered the following status checks when analyzing this PR:

linter: passed
azure: passed

Thus the PR was passing and merged! Have a great day!

mattwthompson commented 5 months ago

Works well enough for us 🙃

Thanks for your work and patience @njzjz - I think people will like this addition

conda-forge / ambertools-feedstock

build MPI #133