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

Better docs for cell, charges, positions #160

Merged
merged 1 commit into from
Jan 29, 2025
Merged

Better docs for cell, charges, positions #160

merged 1 commit into from
Jan 29, 2025

Conversation

PicoCentauri
Copy link
Contributor

@PicoCentauri PicoCentauri commented Jan 28, 2025

The shapes and the contents of these parameters are not clear in the current version. Now we (hopefully) use the same docstring for all of these.

I also used lower cases for torch.tensor in the docstring to be consistent with other types like list, dict, etc.

For a review it is enough to check if these lines are okay

        :param charges: torch.Tensor of shape ``(n_channels, len(positions))``
             containaing the atomic (pseudo-)charges. ``n_channels`` is the number of
             charge channels the potential should be calculated. For a standard potential
             ``n_channels = 1``. If more than one "channel" is provided multiple
             potentials for the same position are computed depending on the charges and
             the potentials.
         :param cell: torch.tensor of shape ``(3, 3)``, where ``cell[i]`` is the i-th basis
             vector of the unit cell
         :param positions: torch.tensor of shape ``(N, 3)`` containing the Cartesian
             coordinates of the ``N`` particles within the supercell.
         :param neighbor_indices: torch.tensor with the ``i,j`` indices of neighbors for

and a slightly shorter version for the charges in some modules.

    :param charges: torch.tensor of shape ``(len(positions, 1))`` containing the atomic
         (pseudo-)charges

Contributor (creator of pull-request) checklist

  • Tests updated (for new features and bugfixes)?
  • Documentation updated (for new features)?
  • Issue referenced (for PRs that solve an issue)?

Reviewer checklist

  • CHANGELOG updated with public API or any other important changes?

📚 Documentation preview 📚: https://torch-pme--160.org.readthedocs.build/en/160/

Sorry, something went wrong.

:param positions: torch.Tensor, Cartesian coordinates of the particles within
the supercell.
:param neighbor_indices: torch.Tensor with the ``i,j`` indices of neighbors for
:param charges: torch.Tensor of shape ``(n_channels, len(positions))``
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be torch.tensor instead of torch.Tensor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should be done.

@E-Rum E-Rum merged commit b22c7a2 into main Jan 29, 2025
13 checks passed
@E-Rum E-Rum deleted the param-docs branch January 29, 2025 12:34
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