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

Use consistent naming scheme across matrix and vector classes #167

Open
superwhiskers opened this issue Jun 19, 2024 · 3 comments
Open

Use consistent naming scheme across matrix and vector classes #167

superwhiskers opened this issue Jun 19, 2024 · 3 comments
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@superwhiskers
Copy link
Collaborator

as was brought up in #164 by @pelesh, the current names of these fields is unclear. they should be renamed to something like owns_gpu_sparsity_pattern_ and owns_gpu_nonzeroes_ (with equivalents for the cpu)

  • rename owns_gpu_data_, owns_gpu_values_, owns_cpu_data_, owns_cpu_values_ to the proposed names
  • rename the methods for setting them on Sparse
  • add methods for getting them?
@superwhiskers superwhiskers added documentation Improvements or additions to documentation enhancement New feature or request labels Jun 19, 2024
@pelesh
Copy link
Collaborator

pelesh commented Jun 20, 2024

I think an easy solution would be to rename owns_*_data_ to owns_*_indices_. Since these are matrix class member variables, I think it would be clear these refer to matrix values and indices. @kswirydo, please chime in.

@pelesh
Copy link
Collaborator

pelesh commented Oct 9, 2024

Some related issues to discuss:

  • getRowData vs. getRowIndices? Using term "row indices" is a sloppy expression because depending on the matrix format these can be row offsets or row indices. Per @maksud, the sloppy language may be all right here because all developers would understand it.
  • @maksud's suggestion for using consistent language in naming methods and member variables: Use word "indices" to refer to matrix indices and offsets, use word "values" to refer to the values of the matrix nonzero elements, and word "data" to refer to the union of "indices" and "values".
  • setMatrixData method name may be misleading. We are assigning pointer values here and not setting matrix data. Consider renaming it to setDataPointers.
  • setUpdated is clear enough.
  • setNewValues should probably be renamed to setValues. It is simpler.
  • Consider changing vector method name setData to setDataPointers.
  • Consider renaming updateData to copyData.

CC @nkoukpaizan

@pelesh pelesh changed the title rename owns_gpu_data_ / owns_gpu_values_ / etc. to owns_gpu_sparsity_pattern_ / owns_gpu_nonzeroes_ / etc. Use consistent naming scheme across matrix and vector classes Oct 9, 2024
@pelesh
Copy link
Collaborator

pelesh commented Oct 9, 2024

CC @kswirydo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants