pydata/sparse wrappers for levels, COO, and CSC/CSR formats

[mtsokol]

Hi @willow-ahrens @hameerabbasi,

Here's a PR with a draft implementation of Python wrappers for Tensor constructors, levels, COO, and CSC formats. Some comments:

• test_coo and test_csc show the construction of fibers using the same code as Hameer's notebook (which adheres to pydata/sparse). Tomorrow I will add CSR and DOK/SparseHash so the whole notebook can be executed (except for GCXS).
• Each wrapper has a _obj attribute which is the underlying Julia object.
• Tensor can be constructed by providing levels description (wrapper classes!) and NumPy/Finch array, or just Finch object directly (jl_data).
• Only simple element-wise operations are executed here.
• There's also an implementation of the todense function that is heavily used in pydata/sparse. Here it's done by constructing a new lvl description with Dense levels only and creating a new Tensor.
• In a separate PR (I think?) I will add CI stages for running those tests.

Questions:

1. Is it possible to access fill_value in Element? I mean the first type parameter in Element{0.0, Float64, Int64}(...)
2. How can I get the shape tuple of Tensor? Right now I obtain shape by iterating through levels and aggregating shape into one tuple.

[willow-ahrens]

This looks awesome! I won't have time for a detailed review until tomorrow, but on first glance this looks great!

[willow-ahrens]

To answer your questions:

1. You can access the fill value by calling default(fbr).
2. You can get the shape tuple by calling size(fbr).

[hameerabbasi]

LGTM overall except the dependency on sparse. I'd invert the depencency, and move those bits to sparse instead (otherwise we have a circular depencency, and that isn't nice).

[mtsokol]

LGTM overall except the dependency on sparse. I'd invert the dependency, and move those bits to sparse instead (otherwise we have a circular dependency, and that isn't nice).

@hameerabbasi sparse is declared only as a tests dependency, as in test suites I want to compare it with sparse, is it Ok?

FYI, I added an initial setup for CI, but it didn't run, I guess GitHub workflows need to be enabled in repo's settings?
Also, I moved pyproject.toml config from poetry to the same as pydata/sparse (I'm not familiar with poetry here). Could we follow pydata/sparse setup for CI and build process?

[hameerabbasi]

sparse is declared only as a tests dependency, as in test suites I want to compare it with sparse, is it Ok?

TBH, I'm not entirely sure how pip will react here. But I guess we can put that in sparse too, if it causes issues, or test against numpy.array_api.

FYI, I added an initial setup for CI, but it didn't run, I guess GitHub workflows need to be enabled in repo's settings?
Also, I moved pyproject.toml config from poetry to the same as pydata/sparse (I'm not familiar with poetry here). Could we follow pydata/sparse setup for CI and build process?

That SGTM, but I guess we should ask @willow-ahrens

@willow-ahrens For context, we follow pre-commit rules for linting and formatting. It's quite nice with auto fixes, even in PRs.

[willow-ahrens]

@mtsokol

sparse is declared only as a tests dependency, as in test suites I want to compare it with sparse, is it Ok?

I think that's fine for now! Eventually we may want to use an independent source of truth, but this is fine for now. I don't think circular dependencies will be an issue because it's not finch that depends on sparse, the test environment depends on sparse. The only issue is that if sparse ever begins to use Finch, then testing finch against sparse is just testing finch against itself.

FYI, I added an initial setup for CI, but it didn't run, I guess GitHub workflows need to be enabled in repo's settings?

Sometimes CI needs to be committed to main before it runs. Maybe make a separate PR for CI and we can merge it, then use this branch to test it?

Also, I moved pyproject.toml config from poetry to the same as pydata/sparse (I'm not familiar with poetry here). Could we follow pydata/sparse setup for CI and build process?

For CI, absolutely let's get that going. For build process, I'm open to this but I'd like to discuss in a meeting. Poetry is a popular option and it's really quite nice. I encourage you to give it a try! What in particular do you want to fix about the build process? What led to pydata/sparse using setuptools-scm? If we do find it necessary, could we change the build system in a separate PR?

@hameerabbasi

For context, we follow pre-commit rules for linting and formatting. It's quite nice with auto fixes, even in PRs.

Yes absolutely, let's get that going in a separate PR and merge it so that we can test it out on this one!

[willow-ahrens]

Here's the guide I was following to use Poetry: https://www.freecodecamp.org/news/how-to-build-and-publish-python-packages-with-poetry/

[willow-ahrens]

Modulo my suggested change to printing, I approve of the new functionality. Please make changes to CI as a separate PR. Please also make changes to the build system as another separate PR. Thanks for helping out with this!

[willow-ahrens]

This should be:

class _Display:
    def __repr__(self):
        return jl.sprint(jl.show, self._obj)
    def __str__(self):
        return jl.sprint(jl.show, jl.MIME("text/plain"), self._obj)

[willow-ahrens]

Once you add CI and pre-commit hooks in a separate PR, I'll make sure they run and that CI must pass before merging.

[willow-ahrens]

I didn't catch this the first time around, but CSR is not Sparse(Dense(...)). Rather, it is the transposition of CSC. Sparse(Dense(...)) is a weird format that looks like

[ 0 0 0 0;
  1 2 3 4;
  0 0 0 0;
  0 0 0 0;
  6 7 8 9]

[hameerabbasi]

From the CI:

--cov-report --cov-report html --cov-report=xml --cov-report=term --cov sparse --cov-config .coveragerc tests

Looks like we need pytest-cov in the test deps.

[mtsokol]

Looks like we need pytest-cov in the test deps.

@hameerabbasi Now tests are running in the CI. One thing - I changed Miniforge variant to the previous Mambaforge as the former one caused 404 error image not found or something similar.

[hameerabbasi]

Suggested change

	miniforge-variant: Mambaforge
	miniforge-version: latest

[hameerabbasi]

Also we should move to setup-miniconda@v3

[mtsokol]

Hi @willow-ahrens @hameerabbasi,

I pushed the latest changes: There are simple COO, CSF, CSR, and CSC wrappers together with Finch Tensor/Levels wrappers.

In terms of row/column major manipulation, instead of reversing dims I did it by reinterpreting the input as column major on entry np.array(input, order="F") and back to row major before returning np.array(result, order="C"). I added an order flag check to test to verify that todense() returns "C"-order numpy array.

[willow-ahrens]

Ah, I see. Let's merge it and make a separate PR to support swizzles.