Skip to content

Update NSVHMEM block for new releases on Github#538

Open
mhrywniak wants to merge 2 commits intoNVIDIA:masterfrom
mhrywniak:nvshmem_gh_mpi
Open

Update NSVHMEM block for new releases on Github#538
mhrywniak wants to merge 2 commits intoNVIDIA:masterfrom
mhrywniak:nvshmem_gh_mpi

Conversation

@mhrywniak
Copy link
Copy Markdown

Releases >= 3.4.5 are published on Github, ensure they get picked up. Fix mpi arg to allow using either True or an explicit MPI_HOME.

I could not run the pydocmd generate command, do I need a different package? uvx -p 3.11 -w pydoc-markdown pydocmd generate seems to expect a .py file instead, and with newer Python versions I get errors about the deprecated/removed imp package.

Pull Request Description

Author Checklist

  • Updated documentation (pydocmd generate) if any docstrings have been modified
  • Passes all unit tests

Releases >= 3.4.5 are published on Github, ensure they get picked up.
Fix `mpi` arg to allow using either True or an explicit MPI_HOME
Copy link
Copy Markdown
Collaborator

@samcmill samcmill left a comment

Choose a reason for hiding this comment

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

Would you please add a test case for version 3.4.5 to exercise the new behavior?

self.__cmake_opts.append('-DNVSHMEM_MPI_SUPPORT=1')
self.__cmake_opts.append('-DMPI_HOME={}'.format(self.__mpi))
#else:
# self.__cmake_opts.append('-DNVSHMEM_MPI_SUPPORT=0')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is the else branch still needed? Or does NVSHMEM now default to that case?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good point - I will double-check the default, but I think the else can go away.

@samcmill
Copy link
Copy Markdown
Collaborator

@jilliebean @sopcao for viz

@samcmill
Copy link
Copy Markdown
Collaborator

Also, since you updated the docstring, the docs will need to be regenerated. I can do that post-merge since it requires having pydocmd installed.

@mhrywniak
Copy link
Copy Markdown
Author

Thanks for the review! I will likely get around to implementing this on Monday.

Also, since you updated the docstring, the docs will need to be regenerated. I can do that post-merge since it requires having pydocmd installed.

I can also regenerate this, but uvx -p 3.11 -w pydoc-markdown pydocmd generate gave me error messages. Do you need a specific pydocmd version installed?

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