Add vector properties

[eriknw]

WIP. This probably closes #2721.

This uses cudf List dtypes to store vectors. When converting a vector column to arrow, the data appears to be on host, so it's unclear how many copies and moves of the data we're doing, but I don't think we have many easy alternatives besides relying on what cudf gives us. In pandas, vector properties are object dtype and stored as numpy arrays.

I think it makes sense for a vector property to be required to be the same length (i.e., if it's added multiple times).

We may want to add a method to convert a vector property to a numpy or cupy array.

• How do we handle null values? Raise? Set to 0? Ignore/skip?
• Should users be able to specify numpy or cupy array (host or device)?

When getting data, should we allow vector properties to be expanded?

Can we create a graph with vector property data?

Should we add a keyword argument to add_vertex_data to say "use all (or the rest) of columns as a vector property of this name"?

Should we allow vector properties to come in already as cupy List dtype?

[BradReesWork]

@eriknw the code looks good, but I do not see any test associated with the changes

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.12@403ee20). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files

@@               Coverage Diff               @@
##             branch-22.12    #2882   +/-   ##
===============================================
  Coverage                ?   60.35%           
===============================================
  Files                   ?      122           
  Lines                   ?     7158           
  Branches                ?        0           
===============================================
  Hits                    ?     4320           
  Misses                  ?     2838           
  Partials                ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[alexbarghi-nv]

@eriknw here are my responses to your questions:

1. Vector properties should be fixed length; if attempting to update an existing vertex property, the length must match or an error should be thrown.
2. Null values should be set to 0.
3. Users should have the choice of a numpy or cupy array, but we can leave numpy support to a future PR.
4. Could you clarify what you mean by "expanding" a vector property?
5. Yes, and could we make that on by default with the default name as "x"?
6. Yes, but let's do that in a future PR.

[eriknw]

Very helpful @alexbarghi-nv, thank you. My replies:

1. Vector properties should be fixed length; if attempting to update an existing vertex property, the length must match or an error should be thrown.

Perfect. This is as implemented.

2. Null values should be set to 0.

I added a fillvalue=None argument. I allow this to be skipped though if data is expected to exist in order to be faster.

3. Users should have the choice of a numpy or cupy array, but we can leave numpy support to a future PR.

K. I'm happy punting on this too and to give users the native object.

4. Could you clarify what you mean by "expanding" a vector property?

"Expand" to return a dataframe instead of an array, which can kind of match how data comes into PropertyGraph. I don't think this is necessary, since it's easy to go from an array to a dataframe.

5. Yes, and could we make that on by default with the default name as "x"?

Sounds good, but I don't want it on by default. Users should specify they want a vector property.

I think I want to support this by adding another keyword vector_property=None, which can be a string for the vector property name. How should this behave though? Should it include properties specified in property_columns, or should it include every other property that isn't included in property_columns or vector_properties. Thoughts?

We could squeeze this functionality into vector_properties (i.e., by passing a string or True to default to "x"), but this complicates typing and is less clear imho.

Edit: upon further thought, I think the most sense for vector_property="x" is to use all other properties as per the original proposal.

6. Yes, but let's do that in a future PR.

Agreed.

[eriknw]

Alright, I think the API and functionality is complete and includes feedback to:

• fill value upon creation of array from vector property
• simplify creating vector properties from "the rest" of the columns

I would like to add a couple more tests to wrap this up.

[VibhuJawa]

Small change, looks good otherwise

[VibhuJawa]

Can we not use list here but numpy or cupy/array library here please

[eriknw]

I think we need to use list here; it's the only way I could get it to work.

[VibhuJawa]

Gotcha, I think that's fine for now, we can revisit this when we start perf tuning if this becomes a problem

[alexbarghi-nv]

👍

[VibhuJawa]

LGTM.

Thanks for working on this Erik , I expect some speed up with this PR on the below workflow. #2925

[alexbarghi-nv]

rerun tests

[BradReesWork]

rerun tests

[BradReesWork]

@gpucibot merge