sampling docstring clarification

[matbryan52]

Throughout the code the sampling argument is referred to as:

    sampling : two float
        Grid sampling in each dimension [1 / Å].

(or similar) but if we look at how the value is interpreted inside Grid:

    def _adjust_extent(self, gpts: tuple, sampling: tuple):
        if (gpts is not None) & (sampling is not None):
            self._extent = tuple(
                (n - 1) * d if e else n * d
                for n, d, e in zip(gpts, sampling, self._endpoint)
            )
            ...

where extent and gpts are documented as

    gpts : two int
        Number of grid points in each dimension.
    extent : two float
        Grid extent in each dimension [Å].

It seems to me that for extent to have units of [Å] then sampling must effectively have units of [Å / gpt] rather than [1 / Å] as noted in the docstring. If it were a reciprocal-space grid then I could understand the docstring as-is, but this same note appears on many real space grids (e.g. GridScan) where reciprocal Angstroms doesn't fit very well (for me).

Perhaps I've misunderstood (or missed an explanation of the choice in the documentation!).

That said in your Overview doc page we have the line "a Potential with a given real-space sampling (in units of Å)", but in the docstring of Potential we find:

    sampling : one or two float, optional
        Sampling of the potential in `x` and `y` [1 / Å]. Provide either "sampling" or "gpts".

[TomaSusi]

Thanks for the careful reading of the docstrings, @matbryan52!

sampling denotes the size of the grid point in real space, ie. 0.04 Å means that grid points are that much apart, so the unit should indeed be [Å / gpt].

This is how it is written in most of the docstrings of the most current version, e.g.
https://abtem.github.io/doc/reference/api/_autosummary/abtem.potentials.iam.Potential.html?highlight=sampling#abtem.potentials.iam.Potential.sampling)

If you are indeed looking at the latest version of the code on GitHub, it seems we have some outdated text to fix.

Btw, not sure if you noticed, but we have an appendix explaining the sampling, is there some more clarification you'd like to see there?
https://abtem.github.io/doc/user_guide/appendix/antialiasing.html?highlight=sampling#sampling-and-antialiasing

[matbryan52]

No problem @TomaSusi. I do think this is still present on main, for example :

abTEM/abtem/potentials/iam.py

Lines 771 to 772 in ef9688f

	sampling : one or two float, optional
	Sampling of the potential in `x` and `y` [1 / Å]. Provide either "sampling" or "gpts".

Thankfully the wording is quite consistent so it should be an easy change.

[jacobjma]

Thank you @matbryan52, for finding this issue in our documentation. I am currently looking through the docs to see if there are other examples. I will fix them for the next version.

[jacobjma]

@matbryan Thanks again for seeing this issue. I am pretty sure I tracked all the instances down and fixed them.

I will close the issue, if you find anything similar please open a new issue, this was very helpful. Thanks again.

[matbryan52]

Thanks @jacobjma, looks good.