[GraphBolt] Improving `ondisk_dataset` tests. #7052

drivanov · 2024-02-01T01:00:13Z

Description

The goal of this PR is to resolve the following 24 warnings that appear when running tests from test_ondisk_dataset.py:

tests/python/pytorch/graphbolt/impl/test_ondisk_dataset.py: 24 warnings
  /usr/local/lib/python3.10/dist-packages/dgl/graphbolt/impl/ondisk_dataset.py:260: DGLWarning: Edge feature is stored, but edge IDs are not saved.
    dgl_warning("Edge feature is stored, but edge IDs are not saved.")

To achieve this goal, I created and started using a new local function `_on_disk_dataset(...) which uses the method already used several times in this file:

    with pytest.warns(
        DGLWarning,
        match="Edge feature is stored, but edge IDs are not saved.",
    ):

Checklist

Please feel free to remove inapplicable items for your PR.

The PR title starts with [$CATEGORY] (such as [NN], [Model], [Doc], [Feature]])
I've leverage the tools to beautify the python and c++ code.
The PR is complete and small, read the Google eng practice (CL equals to PR) to understand more about small PR. In DGL, we consider PRs with less than 200 lines of core code change are small (example, test and documentation could be exempted).
To the best of my knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

dgl-bot · 2024-02-01T01:00:44Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-01T01:00:56Z

Commit ID: 9c70264

Build ID: 1

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

dgl-bot · 2024-02-01T01:15:19Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-01T01:15:30Z

Commit ID: fa40d8c

Build ID: 2

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

dgl-bot · 2024-02-01T17:17:22Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-01T17:17:34Z

Commit ID: 5a1c89c

Build ID: 3

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

… ondisk_dataset

dgl-bot · 2024-02-01T17:25:06Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-01T17:25:18Z

Commit ID: e11672d

Build ID: 4

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

Rhett-Ying

I think it's better to explicitly capture warnings directly in the testcase. and check if we really need to enable include_original_edge_id or remove edge feature before adding such capture. We only capture for those where warning is inevitable.

dgl-bot · 2024-02-02T18:41:19Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-02T18:41:30Z

Commit ID: 05ae715

Build ID: 5

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

drivanov · 2024-02-02T19:45:35Z

@Rhett-Ying : Sorry, I'm not sure I understand what you're talking about. We already have similar captures of that warning here and here. They are generated only for include_original_edge_id=False. I created a generic function for these two and 24 other cases.

Note that I'm not simply suppressing this warning even though it won't be generated. With the following lines:

    with pytest.warns(
        DGLWarning,
        match="Edge feature is stored, but edge IDs are not saved.",
    ):

the program expects that this warning will be generated, therefore it should be considered as inevitable

Moreover, if the program expects to see, but won't see, this warning for some test, that test will fail with the following error message:

================================================================================= short test summary info =================================================================================
FAILED tests/python/pytorch/graphbolt/impl/test_ondisk_dataset.py::test_OnDiskDataset_preprocess_auto_force_preprocess - Failed: DID NOT WARN. No warnings of type (<class 'dgl.base.DGLWarning'>,) were emitted.

I just got it with the following changes:

        # 4. Change nothing.
        with pytest.warns(
            DGLWarning,
            match="Edge feature is stored, but edge IDs are not saved.",
        ):
            preprocessed_metadata_path = (
                gb.ondisk_dataset.preprocess_ondisk_dataset(
                    test_dir, include_original_edge_id=True
                )
            )

I made to test_OnDiskDataset_preprocess_auto_force_preprocess. As you could see, here include_original_edge_id=True, therefore, this warning should not have been generated, and when it was not captured, this test failed.

Rhett-Ying · 2024-02-04T07:13:19Z

@drivanov Sorry for the vagueness. What I mean is if we mean to verify such warning is thrown or inevitable, then directly wrap with with pytest.warns() instead of a wrapper like _ondist_xx() you did. For other testcases that throw warning, we just remove the edge feature or specify include_original_eids=True to eliminate the warnings.

… ondisk_dataset

dgl-bot · 2024-02-05T19:41:38Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-05T19:41:49Z

Commit ID: fc120724e2bd8d6e672c4899937eb7b4eaffde95

Build ID: 6

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

dgl-bot · 2024-02-05T19:51:02Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-05T19:51:14Z

Commit ID: c9e93ad40049222c414a017a1dc43501881be2b1

Build ID: 7

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

dgl-bot · 2024-02-05T20:01:37Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-05T20:01:50Z

Commit ID: 44b3c79

Build ID: 8

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

drivanov · 2024-02-05T20:03:31Z

@Rhett-Ying : I have implemented the changes you suggested.

Rhett-Ying

please fix test failure running on Windows.

dgl-bot · 2024-02-07T17:15:15Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-07T17:15:28Z

Commit ID: 47fdb4a

Build ID: 12

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

Rhett-Ying · 2024-02-08T01:01:19Z

@dgl-bot

dgl-bot · 2024-02-08T01:42:22Z

Commit ID: 47fdb4a

Build ID: 13

Status: ❌ CI test failed in Stage [Torch CPU (Win64) Unit test].

Report path: link

Full logs path: link

… ondisk_dataset

dgl-bot · 2024-02-08T16:32:48Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-08T16:33:00Z

Commit ID: e518992db3f70e2b4a8129d6f36ce6bb6e75dde3

Build ID: 14

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

dgl-bot · 2024-02-08T16:35:35Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-08T16:35:46Z

Commit ID: 0283dcc

Build ID: 15

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

… ondisk_dataset

dgl-bot · 2024-02-08T16:48:45Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-08T16:48:57Z

Commit ID: b8fc709

Build ID: 16

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

Rhett-Ying · 2024-02-09T01:59:10Z

@dgl-bot

Rhett-Ying · 2024-02-09T02:04:13Z

this PR could be abandoned if #7110 is merged.

dgl-bot · 2024-02-09T02:57:15Z

Commit ID: 7039a66f3d4715de2030d79db5756167944e8f70

Build ID: 17

Status: ❌ CI test failed in Stage [Torch CPU (Win64) Unit test].

Report path: link

Full logs path: link

dgl-bot · 2024-02-09T17:00:30Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-09T17:00:42Z

Commit ID: d945042

Build ID: 18

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

dgl-bot · 2024-02-14T17:41:01Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-14T17:41:13Z

Commit ID: d6f3462

Build ID: 19

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

dgl-bot · 2024-02-21T18:23:40Z

Not authorized to trigger CI. Please ask core developer to help trigger via issuing comment:

@dgl-bot

dgl-bot · 2024-02-21T18:23:53Z

Commit ID: f0b70c7

Build ID: 20

Status: ❌ CI test failed in Stage [Authentication].

Report path: link

Full logs path: link

[GraphBolt] Improving ondisk_dataset tests.

9c70264

Fixing lint problems.

fa40d8c

Merge branch 'master' into ondisk_dataset

5a1c89c

drivanov added 2 commits February 1, 2024 09:18

Fixing lint problem.

8134723

Merge branch 'ondisk_dataset' of https://github.com/drivanov/dgl into…

e11672d

… ondisk_dataset

Rhett-Ying requested changes Feb 2, 2024

View reviewed changes

Merge branch 'master' into ondisk_dataset

05ae715

drivanov added 2 commits February 5, 2024 11:35

Changes suggested by @Rhett-Ying

759f446

Merge branch 'ondisk_dataset' of https://github.com/drivanov/dgl into…

982537c

… ondisk_dataset

Fixing lint problems

7f46f22

Merge branch 'master' into ondisk_dataset

44b3c79

Rhett-Ying reviewed Feb 7, 2024

View reviewed changes

Merge branch 'master' into ondisk_dataset

47fdb4a

drivanov added 2 commits February 8, 2024 08:27

Changes suggested by @Rhett-Ying

fc93ed6

Merge branch 'ondisk_dataset' of https://github.com/drivanov/dgl into…

894b0c3

… ondisk_dataset

Merge branch 'master' into ondisk_dataset

0283dcc

drivanov added 2 commits February 8, 2024 08:43

Fixing lint problems

9548212

Merge branch 'ondisk_dataset' of https://github.com/drivanov/dgl into…

b8fc709

… ondisk_dataset

Rhett-Ying approved these changes Feb 9, 2024

View reviewed changes

Merge branch 'master' into ondisk_dataset

d945042

Merge branch 'master' into ondisk_dataset

d6f3462

Merge branch 'master' into ondisk_dataset

f0b70c7

[GraphBolt] Improving ondisk_dataset tests. #7052

Are you sure you want to change the base?

[GraphBolt] Improving ondisk_dataset tests. #7052

Conversation

drivanov commented Feb 1, 2024

Description

Checklist

Changes

dgl-bot commented Feb 1, 2024

dgl-bot commented Feb 1, 2024

dgl-bot commented Feb 1, 2024

dgl-bot commented Feb 1, 2024

dgl-bot commented Feb 1, 2024

dgl-bot commented Feb 1, 2024

dgl-bot commented Feb 1, 2024

dgl-bot commented Feb 1, 2024

Rhett-Ying left a comment

Choose a reason for hiding this comment

dgl-bot commented Feb 2, 2024

dgl-bot commented Feb 2, 2024

drivanov commented Feb 2, 2024

Rhett-Ying commented Feb 4, 2024

dgl-bot commented Feb 5, 2024

dgl-bot commented Feb 5, 2024

dgl-bot commented Feb 5, 2024

dgl-bot commented Feb 5, 2024

dgl-bot commented Feb 5, 2024

dgl-bot commented Feb 5, 2024

drivanov commented Feb 5, 2024

Rhett-Ying left a comment

Choose a reason for hiding this comment

dgl-bot commented Feb 7, 2024

dgl-bot commented Feb 7, 2024

Rhett-Ying commented Feb 8, 2024

dgl-bot commented Feb 8, 2024

dgl-bot commented Feb 8, 2024

dgl-bot commented Feb 8, 2024

dgl-bot commented Feb 8, 2024

dgl-bot commented Feb 8, 2024

dgl-bot commented Feb 8, 2024

dgl-bot commented Feb 8, 2024

Rhett-Ying commented Feb 9, 2024

Rhett-Ying commented Feb 9, 2024

dgl-bot commented Feb 9, 2024

dgl-bot commented Feb 9, 2024

dgl-bot commented Feb 9, 2024

dgl-bot commented Feb 14, 2024

dgl-bot commented Feb 14, 2024

dgl-bot commented Feb 21, 2024

dgl-bot commented Feb 21, 2024

[GraphBolt] Improving `ondisk_dataset` tests. #7052

[GraphBolt] Improving `ondisk_dataset` tests. #7052