Skip to content
This repository has been archived by the owner on Sep 18, 2024. It is now read-only.

Resolve comments in PR 1571 #1590

Merged
merged 6 commits into from
Oct 14, 2019
Merged

Resolve comments in PR 1571 #1590

merged 6 commits into from
Oct 14, 2019

Conversation

liuzhe-lz
Copy link
Contributor

No description provided.

@liuzhe-lz liuzhe-lz changed the base branch from dev-mc to master October 9, 2019 03:38
import torch
import torch.nn.functional as F
import tensorflow as tf
import nni.compression.tensorflow as tf_compressor
import nni.compression.torch as torch_compressor
Copy link
Contributor

Choose a reason for hiding this comment

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

this import order cannot pass pylint

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can.

@@ -32,8 +32,8 @@ class AGP_Pruner(Pruner):
"""
def __init__(self, config_list):
"""
Configure Args
initial_sparsity:
Configure Args:
Copy link
Contributor

Choose a reason for hiding this comment

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

please follow the docstring format like other tuners?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.
Followed format used in msg_dispatcher.handle_report_metric_data

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this format has some differences with the format that we used(We used the format like xgboost). I not sure about that. @leckie-chn

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This does not document the parameter list, instead it documents the fields of one parameter.

@liuzhe-lz liuzhe-lz requested a review from squirrelsc October 11, 2019 02:59
@ultmaster
Copy link
Contributor

Would you add a link to compressor docs in table of contents? Currently, there's no way to see compressor other than searching for it.

Copy link
Member

@scarlett2018 scarlett2018 left a comment

Choose a reason for hiding this comment

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

Let's postpone the known doc problem to next release, please create an issue to track it.

@liuzhe-lz liuzhe-lz merged commit d6b61e2 into microsoft:master Oct 14, 2019
@liuzhe-lz liuzhe-lz deleted the dev-mc-2 branch November 2, 2019 09:00
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.

6 participants