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

Rename subgrid to localgrid #93

Closed
tovrstra opened this issue Mar 3, 2020 · 0 comments · Fixed by #105
Closed

Rename subgrid to localgrid #93

tovrstra opened this issue Mar 3, 2020 · 0 comments · Fixed by #105
Assignees
Labels
enhancement New feature or request

Comments

@tovrstra
Copy link
Member

tovrstra commented Mar 3, 2020

This is something I intended to carry out shortly after #87 but I got carried away by other things.

The name "subgrid" seems to imply that it will always return a subset of the current grid, which is correct for aperiodic grids. In the case of a periodic grid, the result may contain portions from neighboring images, which could lead to more grid points in the subgrid than the parent grid. The name "localgrid" would make more sense because the algorithm takes a center and a radius as arguments.

@tovrstra tovrstra added the enhancement New feature or request label Mar 3, 2020
@tovrstra tovrstra self-assigned this Mar 3, 2020
tovrstra added a commit to tovrstra/grid that referenced this issue Aug 11, 2020
Fixes theochem#93.

The function get_subgrid can return larger grids than the original
one in case of periodic grids, typically when the cutoff sphere
contains multiple unit cells. The resulting grid is therefore not
necessarily a "sub"grid. This function does always make a local
grid, centered around some piont, making "local"grid is more
appropriate.

In some cases, the term subgrid was used in another context, e.g.
for an atomic grid in a molecular grid. These were not renamed.

At the momemt, the following renamings were done:

- `subgrid` -> `localgrid`
- `subfn` -> `localfn`
- `SubGrid` -> `SubGrid`

and a few variants of these. One could argue that the first two would
look better with an extra underscore after `local`.
tczorro pushed a commit that referenced this issue Aug 12, 2020
Fixes #93.

The function get_subgrid can return larger grids than the original
one in case of periodic grids, typically when the cutoff sphere
contains multiple unit cells. The resulting grid is therefore not
necessarily a "sub"grid. This function does always make a local
grid, centered around some piont, making "local"grid is more
appropriate.

In some cases, the term subgrid was used in another context, e.g.
for an atomic grid in a molecular grid. These were not renamed.

At the momemt, the following renamings were done:

- `subgrid` -> `localgrid`
- `subfn` -> `localfn`
- `SubGrid` -> `SubGrid`

and a few variants of these. One could argue that the first two would
look better with an extra underscore after `local`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant