Skip to content
This repository was archived by the owner on Sep 12, 2025. It is now read-only.

refactor: gnn_dataloader#27

Merged
JiahangXu merged 8 commits intomicrosoft:dev/setup-refactorfrom
xuan301:main
Sep 26, 2021
Merged

refactor: gnn_dataloader#27
JiahangXu merged 8 commits intomicrosoft:dev/setup-refactorfrom
xuan301:main

Conversation

@xuan301
Copy link
Copy Markdown
Contributor

@xuan301 xuan301 commented Sep 15, 2021

No description provided.

@ghost
Copy link
Copy Markdown

ghost commented Sep 15, 2021

CLA assistant check
All CLA requirements met.

@Lynazhang Lynazhang requested a review from JiahangXu September 15, 2021 08:51
global op_num
op_num = len(self.op_types)

def download_data(self):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use .bench_dataset.bench_dataset() to download and get the data.

Comment thread nn_meter/dataset/gnn_dataloader.py Outdated


class GNNDataset(torch.utils.data.Dataset):
def __init__(self, data_dir='./dataset', train=True, device='cpu', split_ratio=0.8):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the data_dir should be default set as user_dataset_folder

Comment thread nn_meter/dataset/gnn_dataloader.py Outdated
Comment thread examples/gnn_for_bench_dataset.ipynb Outdated
"source": [
"# Latency Dataset - GNN Model\n",
"\n",
"This example will demonstrate our ability to predict latency with data from NN-meter through GNN. We will first build our GNN model, which is constructed based on GraphSAGE, and maxpooling is selected as out pooling method. Next, we will start training after the data is loaded.\n",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example will demonstrate our ability to predict latency with data from NN-meter through GNN." Change the sentence to "Considering the dataset is encoded in a graph format, here is an example of using GNN to predict the model latency with the bench dataset. GNNDataset and GNNDataloader in nn_meter/dataset/gnn_dataloader.py build the model structure of the Dataset in .jsonl format into our required Dataset and Dataloader."

Comment thread examples/gnn_for_bench_dataset.ipynb Outdated
"\n",
"lr_scheduler = CosineAnnealingLR(opt, T_max=EPOCHS)\n",
"loss_sum = 0\n",
"for epoch in range(EPOCHS):\n",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add more comment to explain the code, such as "start training", "start validation", "save the best model", etc.

Comment thread nn_meter/dataset/gnn_dataloader.py Outdated
err_str = "Not supported device type"
assert device in hws, err_str
self.device = device
self.data_dir = data_dir
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the setting from .bench_dataset, the dataset will downloaded to __user_dataset_folder__ by default. Thus users cannot get access to the downloaded dataset if users set the init parameter data_dir to other path. The API of .bench_dataset and GNNDataset.__init__ should be unified, either both have data_dir or both not.

self.raw_data = {}
self.name_list = []
self.latencies = {}
self.download_data()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest using self.data_dir = self.download_data() and return the dataset path in self.download_data.

Comment thread nn_meter/dataset/gnn_dataloader.py Outdated
self.construct_attrs()
self.name_list = list(
filter(lambda x: x in self.latencies, self.name_list))
global op_num
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this parameter should be global?

@xuan301 xuan301 changed the base branch from main to dev/setup-refactor September 26, 2021 02:58
@JiahangXu JiahangXu merged commit 1d141ce into microsoft:dev/setup-refactor Sep 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants