chapel icon indicating copy to clipboard operation
chapel copied to clipboard

Naming of CSR/CSC-related things (LayoutCS and the CS record)

Open bradcray opened this issue 6 months ago • 13 comments

In past PRs, we've converted most of our standard user-facing distributions from classes to records, such that:

  • class Block defined in the BlockDist module became record blockDist
  • class Cyclic defined in the CyclicDist module became record cyclicDist

and so on… Notably missing in that effort were the layouts, particularly the CSR/CSC layout used for sparse computation as an alternative to the default COO layout. We simply didn't get to it due to lack of time. In #25812, I'm working on making up for that.

This issue asks what we should name the module and layout record going forward. Sparse is still an unstable feature in Chapel, so technically we don't need to stabilize these names now, but if we get them right now, it'll be fewer changes later as sparse becomes stable.

Currently, the module is named LayoutCS and the class is simply named CS (with arguments, such as compressRows=true or compressRows=false to select between CSR or CSC).

Q1: What should we name the module?

We could retain the current name, but I was noticing that it swaps the order of the Distribution/Layout classification compared to the distributions, so might be more consistent to take this opportunity to change it to CSLayout or CsLayout or CompressedSparseLayout or … This also _may_help with transitioning code between the old and the new, though that shouldn't be a primary consideration.

Q2: What should we name the record?

If we simply changed the current name to a style guide appropriate name, we could just change it to cs. If we followed the approach of the distributions, we could name it csLayout or compressedSparseLayout or layoutCS or whatever the de-capitalized version of the module name is.

Q2a: Or, should we not expose the record name directly, only through aliases?

Users in the field will be most familiar with CSR/CSC as concepts, not some sort of cs(rows=true) initializer like the one we have now. So we could also imagine not worrying about the name of the record and just coming up with names for the aliases to the record, like:

  • 'csr', 'csrLayout', 'CSR' (technically a style guide violation)
  • 'csc', 'cscLayout', 'CSC' (" " ")

[I was originally thinking of this as a nicety to put on top of the record, so not necessary to discuss from the start, but Engin pointed out below that we don't have to tell the users what the record name is, which is a great point].

bradcray avatar Aug 26 '24 22:08 bradcray