Quickstart and Basic usage example #123
Conversation
| potentials_aperiodic = calculator._compute_rspace( | ||
| charges, neighbor_indices_aperiodic, neighbor_distances_aperiodic | ||
| ) |
There was a problem hiding this comment.
Should we maybe define a new calculatur here. Which is just torchpme.Calculator and a new potential with 0 smearing? This might be cleaner compared to using private methods.
There was a problem hiding this comment.
I agree, but also feel that this should be a separate PR. I can open an issue about this. Until then, we can either keep this example using the private function, or even just remove it altogether.
There was a problem hiding this comment.
I think you can already do it
from torchpme import Calculator
calculator = Calculator(potential=CoulombPotential(smearing=0))
calculator.forward(...)even though I agree the same is not perfect;y indicating that this is for aperiodic structures.
There was a problem hiding this comment.
This doesn't work, because in its current version, Calculator's forward requires an implementation of the reciprocal space sum. So this just raises an error. I have kept the example for now, but made the comments more detailed to reduce potential confusion about why this has been implemented like this.
There was a problem hiding this comment.
Arghh yes. Can you open an issue. We should fix this to allow easy usage of Real space Coulomb!
|
Thanks for the changes you made and the comments @PicoCentauri . I have now generally tried to improve the comments, with suggestions coming from our beta tester as well. The issue regarding the aperiodic calculator goes beyond the intention of this PR, which is why I kept the example simple on that front. |
|
Good addition for the quickstart @PicoCentauri . Should we perhaps also reference the newly added |
|
Yes, I will update the link! |
| cell = 8 * torch.eye(3) | ||
| charges = torch.tensor([[1.0]]) | ||
|
|
||
| # No neighbors for a single atom; use `vesin` for neighbors if needed |
There was a problem hiding this comment.
If it would be better to say how to use vesin here?
There was a problem hiding this comment.
Good point and I thought about it as well. In the end I decided against to keep the quickstart short and only about torch-pme; details will be shown in the basic usage tutorial.
There was a problem hiding this comment.
This is a good point though. I added some comments in basic-usage.py to explain why we use the neighbor list from vesin instead of some other package.
We have good examples explaining various aspects of torch-pme. But we lack a very basic usage example for beginners. This PR adds such a tutorial which will be displayed prominently just after the installation page and refer users to the examples and API references in the end.
Contributor (creator of pull-request) checklist
Reviewer checklist
📚 Documentation preview 📚: https://torch-pme--123.org.readthedocs.build/en/123/