[2/n] `MultiEmbeddingTensor`: Add `MultiEmbeddingTensor` #181

akihironitta · 2023-11-06T05:32:23Z

Adds MultiEmbeddingTensor as part of #88.

Follow-up tasks:

Support slicing and indexing
Add stype.embedding and EmbeddingTensorMapper
Add an example
~~Refactor this with MultiNestedTensor~~ being addressed via [1/n] MultiEmbeddingTensor: Restructure and clean up MultiNestedTensor #183.

torch_frame/data/multi_embedding_tensor.py

weihua916

Thanks for starting. I think we can create a base class to share basic logics with MultiNestedTensor. For MultiEmbeddingTensor, I think you can first implement MultiEmbeddingTensor.from_tensor_list and make sure met[i, j] is working properly. Then, we can gradually build up advanced indexing and concatenation in separate PRs.

test/data/test_multi_embedding_tensor.py

torch_frame/data/multi_embedding_tensor.py

yiweny

Thank you for your work. I think there are some small bugs and edge case handling that need to be resolved in this pr.
Also, I think it might be better to create a base class for this and MultiNestedTensor.

torch_frame/data/multi_embedding_tensor.py

yiweny · 2023-11-06T20:33:22Z

test/data/test_multi_embedding_tensor.py

+    num_rows = 3
+    num_cols = 2
+    offset = torch.tensor([0, 2, 4])
+    values = torch.tensor([


Use rand to generate tensor.

I prefer this way to random tensors for readability, but I can replace this if you'd advocate for it

test/data/test_multi_embedding_tensor.py

codecov · 2023-11-07T01:19:51Z

Codecov Report

Merging #181 (bab9c1f) into master (a6a3bae) will increase coverage by 0.18%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #181      +/-   ##
==========================================
+ Coverage   91.86%   92.05%   +0.18%     
==========================================
  Files         105      107       +2     
  Lines        4376     4480     +104     
==========================================
+ Hits         4020     4124     +104     
  Misses        356      356

Files	Coverage Δ
test/data/test_multi_embedding_tensor.py	`100.00% <100.00%> (ø)`
torch_frame/data/__init__.py	`100.00% <100.00%> (ø)`
torch_frame/data/multi_embedding_tensor.py	`100.00% <100.00%> (ø)`

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

Rename _to_positive_index _normalize_index [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci unused imports docs update

[pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci

for more information, see https://pre-commit.ci

akihironitta

@yiweny @rusty1s @weihua916 @zechengz Ready for your second review ;)

into aki/feat-met

weihua916

LGM, thanks!

torch_frame/data/multi_embedding_tensor.py

for more information, see https://pre-commit.ci

into aki/feat-met

yiweny · 2023-11-08T05:03:28Z

torch_frame/data/multi_embedding_tensor.py

+
+    def __getitem__(
+        self,
+        index: Any,


Suggested change

index: Any,

index: IndexSelectType,

Will address in a follow-up!

akihironitta added 0 - Priority P0 feature data labels Nov 6, 2023

akihironitta self-assigned this Nov 6, 2023

akihironitta requested review from weihua916, zechengz and yiweny November 6, 2023 05:32

akihironitta force-pushed the aki/feat-met branch 3 times, most recently from eaec2b3 to ed5e6e3 Compare November 6, 2023 05:51

zechengz reviewed Nov 6, 2023

View reviewed changes

torch_frame/data/multi_embedding_tensor.py Show resolved Hide resolved

torch_frame/data/multi_embedding_tensor.py Outdated Show resolved Hide resolved

rusty1s reviewed Nov 6, 2023

View reviewed changes

weihua916 reviewed Nov 6, 2023

View reviewed changes

test/data/test_multi_embedding_tensor.py Show resolved Hide resolved

torch_frame/data/multi_embedding_tensor.py Show resolved Hide resolved

torch_frame/data/multi_embedding_tensor.py Show resolved Hide resolved

yiweny requested changes Nov 6, 2023

View reviewed changes

akihironitta mentioned this pull request Nov 6, 2023

[1/n] MultiEmbeddingTensor: Restructure and clean up MultiNestedTensor #183

Merged

akihironitta changed the base branch from master to aki/refactor-tensors November 7, 2023 01:12

akihironitta force-pushed the aki/feat-met branch from ed5e6e3 to 531c1ed Compare November 7, 2023 01:19

akihironitta force-pushed the aki/refactor-tensors branch from 6c1f2c9 to aaf88a9 Compare November 7, 2023 02:23

akihironitta force-pushed the aki/feat-met branch from 531c1ed to efc5e34 Compare November 7, 2023 02:36

akihironitta changed the title ~~Add MultiEmbeddingTensor~~ [2/n] MultiEmbeddingTensor: Add MultiEmbeddingTensor Nov 7, 2023

akihironitta marked this pull request as draft November 7, 2023 06:00

akihironitta mentioned this pull request Nov 7, 2023

Handle differnt dtype in MultiNestedTensor/MultiEmbeddingTensor #184

Open

akihironitta force-pushed the aki/feat-met branch from efc5e34 to d689867 Compare November 7, 2023 07:23

akihironitta force-pushed the aki/refactor-tensors branch from aaf88a9 to 9ac1e69 Compare November 7, 2023 07:26

Restructure and clean up MultiNestedTensor

1576bbe

Rename _to_positive_index _normalize_index [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci unused imports docs update

akihironitta force-pushed the aki/refactor-tensors branch from 9ac1e69 to 1576bbe Compare November 7, 2023 07:37

akihironitta force-pushed the aki/feat-met branch from bbdf3cd to 78e6cc6 Compare November 7, 2023 07:39

akihironitta added 2 commits November 8, 2023 03:16

Address reviews

562d31a

Address reviews

3e25ce7

akihironitta and others added 3 commits November 8, 2023 03:17

Address reviews

b100206

Add MultiEmbeddingTensor

fd16cdb

[pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci

[pre-commit.ci] auto fixes from pre-commit.com hooks

9255f0e

for more information, see https://pre-commit.ci

akihironitta force-pushed the aki/feat-met branch from 78e6cc6 to 9255f0e Compare November 8, 2023 03:23

Base automatically changed from aki/refactor-tensors to master November 8, 2023 03:41

akihironitta added 3 commits November 8, 2023 03:42

update

180bbbe

update

e0d9980

update

13f5120

akihironitta commented Nov 8, 2023

View reviewed changes

Merge branch 'master' into aki/feat-met

0eed6e9

akihironitta requested review from yiweny, zechengz, weihua916 and rusty1s November 8, 2023 04:13

akihironitta marked this pull request as ready for review November 8, 2023 04:13

akihironitta added 3 commits November 8, 2023 04:14

update

d3718a4

update

14c3476

Merge branch 'aki/feat-met' of https://github.com/pyg-team/pytorch-frame

5c1176a

into aki/feat-met

weihua916 approved these changes Nov 8, 2023

View reviewed changes

torch_frame/data/multi_embedding_tensor.py Outdated Show resolved Hide resolved

torch_frame/data/multi_embedding_tensor.py Outdated Show resolved Hide resolved

akihironitta and others added 4 commits November 8, 2023 04:39

address comments from weihua

2f97446

[pre-commit.ci] auto fixes from pre-commit.com hooks

41be767

for more information, see https://pre-commit.ci

address comments from weihua

9f4d290

Merge branch 'aki/feat-met' of https://github.com/pyg-team/pytorch-frame

bab9c1f

into aki/feat-met

akihironitta enabled auto-merge (squash) November 8, 2023 04:46

yiweny approved these changes Nov 8, 2023

View reviewed changes

akihironitta merged commit b9bf8a0 into master Nov 8, 2023

akihironitta deleted the aki/feat-met branch November 8, 2023 05:04

akihironitta mentioned this pull request Nov 9, 2023

[3/n] MultiEmbeddingTensor: Support cat #193

Merged

akihironitta mentioned this pull request Nov 18, 2023

Fix [:, int] indexing in MultiEmbeddingTensor #230

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2/n] `MultiEmbeddingTensor`: Add `MultiEmbeddingTensor` #181

[2/n] `MultiEmbeddingTensor`: Add `MultiEmbeddingTensor` #181

akihironitta commented Nov 6, 2023 •

edited

Loading

weihua916 left a comment •

edited

Loading

yiweny left a comment

yiweny Nov 6, 2023

akihironitta Nov 8, 2023

codecov bot commented Nov 7, 2023 •

edited

Loading

akihironitta left a comment

weihua916 left a comment

yiweny Nov 8, 2023

akihironitta Nov 8, 2023

[2/n] MultiEmbeddingTensor: Add MultiEmbeddingTensor #181

[2/n] MultiEmbeddingTensor: Add MultiEmbeddingTensor #181

Conversation

akihironitta commented Nov 6, 2023 • edited Loading

weihua916 left a comment • edited Loading

Choose a reason for hiding this comment

yiweny left a comment

Choose a reason for hiding this comment

yiweny Nov 6, 2023

Choose a reason for hiding this comment

akihironitta Nov 8, 2023

Choose a reason for hiding this comment

codecov bot commented Nov 7, 2023 • edited Loading

Codecov Report

akihironitta left a comment

Choose a reason for hiding this comment

weihua916 left a comment

Choose a reason for hiding this comment

yiweny Nov 8, 2023

Choose a reason for hiding this comment

akihironitta Nov 8, 2023

Choose a reason for hiding this comment

[2/n] `MultiEmbeddingTensor`: Add `MultiEmbeddingTensor` #181

[2/n] `MultiEmbeddingTensor`: Add `MultiEmbeddingTensor` #181

akihironitta commented Nov 6, 2023 •

edited

Loading

weihua916 left a comment •

edited

Loading

codecov bot commented Nov 7, 2023 •

edited

Loading