chapel
chapel copied to clipboard
Change CSR/CSC layout class 'CS' to `csrLayout`/`cscLayout` records and rename `LayoutCS` module
As part of our ongoing conversion of domain maps from classes to records, this PR changes the CS
class that has been used to represent sparse CSR/CSC domains and arrays into a pair of records, csrLayout
and cscLayout
. Simultaneously, it renames the LayoutCS.chpl
module to CompressedSparseLayout.chpl
to (a) move Layout
from a prefix to a suffix, similar to the other distributions and (b) make the name a bit more self-descriptive (since 'CS' isn't generally recognized as being the prefix of CSR/CSC). This approach also made it easy to generate reasonable deprecation warnings for the old approach by essentially starting from a code clone of the LayoutCS module and attaching @deprecated
attributes to the module and class declarations.
Despite these improvements, I've left the new module and record type as @unstable
since sparse as a whole is unstable and the routines used here haven't been reviewed in any detail.
With this change, I believe DefaultDist()
is the only class-based domain map remaining and the only thing standing in the way of removing the dmap
type (?).
Most of the diff here is updating module and test code from dmapped new dmap(new CS())
to one of dmapped new csrLayout()
or dmapped new cscLayout()
, preserving the sortedIndices
argument when used.
For the implementation of the record itself, I took a similar approach to the records defining blockDist, cyclicDist, etc. by preserving the old classes and simply wrapping them in the record, combined with using a helper record as a means of providing common capabilities between the csrLayout/cscLayout record types and, hopefully, DefaultDist()
once it's converted to a record.
Originally, I'd hoped to use a single csLayout
record to define both csrLayout
and cscLayout
as partial instantiations of it, but I had problems coming up with an approach that caused the two types to work as both a fully-defaulted type (needing no arguments) and a generic type that could accept them. The results of these failed attempts are noted in comments, as I think that's worth returning to… In part to remove a bit of repetition from the code, and in part because it feels like an important case for the language to be able to support. I'll also plan to file an issue or issues about what I found there.
As a result, CompressedSparseLayout.chpl is essentially:
- a code clone of LayoutCS.chpl
- promoted the module from a
prototype
to a true module by addingthrows
decorators to thedsiSerialWrite()
calls - renamed
LayoutCSDefaultToSorted
tocsLayoutDefaultToSorted
to better math the module name - updated the
isCSType()
query to work with the new records - changed the
CS
class into aCSImpl
class that the records wrap- removed its default for
compressRows
in the process because it's unnecessary and could just mask mistakes
- removed its default for
- added a
chpl_layoutHelper
record that is similar to thechpl_distHelp
record used for the Block, Cyclic, etc. conversions, whose goal is to factor things that used to be inherited fromBaseDist
into a common record that could be used by multiple types; currently it's only this one, but maybeDefaultDist
will be able to make use of it as well? Or maybe it's overkill / putting the cart before the horse. - added the
csrLayout
/cscLayout
records, which simply define initializers and==
/!=
operators - preserved the aforementioned unsuccessful attempt to define a
csLayout
type that could be instantiated twice to remove some of the code duplication involved in having two records above - added a
dsiGetDist()
call to convert a class back into a distribution record value
Taking stock of other changes required:
- modules/layouts/LayoutCS.chpl: marked the module and
CS
class as deprecated - modules/Makefile, fixDistDocs.perl: added new entry to ensure that the new module is documented
- dists/BlockDist.chpl, dists/SparseBlockDist.chpl, packages/LinearAlgebra.chpl: updated to use the new module and type names
- test/*.chpl: For all tests, I essentially converted:
- uses/imports of
LayoutCS
intoCompressedSparseLayout
-
dmap(new CS())
anddmap(new CS(compressRows=true))
intocsrLayout()
-
dmap(new CS(compressRows=false))
intocscLayout()
- in cases like the above that passed
sortIndices
, I preserved those arguments
- in cases like the above that passed
- references to
unmanaged CS
intocsrLayout
- cases that chose between the two using a param variable into a conditional that chooses between the types
- cases that have to support both
cs*Layout
andDefaultDist
to wrap the latter in a dmap+unmanaged and the former not - cases that extended the
CS
class to extendCSImpl
instead
- uses/imports of
- test/*.compopts: For cases that passed in
CS
as aconfig type
, I changed tocsrLayout
TODO:
- [ ] check docs and comments
- [ ] add test locking in deprecations for old module/class
- [ ] file issues for shared record pattern that didn't work out
- [ ] look at what's required to keep Arkouda running