-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Code modification for testcases of various network models in directory example #12498
Conversation
@luobao-intel thanks for your contribution. Could you update the PR title to be inline with what exactly this is fixing? @mxnet-label-bot[pr-awaiting-testing] |
@luobao-intel Could you check the build failure so that we can proceed with the review @ThomasDelteil @safrooze for review |
@luobao-intel Requesting an update on the PR, any progress on the build failures? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is more confusing and not very intuitive for the user to say, gpus= any -ve number for CPU and any +ve number for GPU.
Maybe you can consider having a parameter like "gpus" where gpus=None by default which means run on CPU. gpus=0 to use gpu(0), gpus=0,1,2,3 to use gpu(0), gpu(1), gpu(2), gpu(3).
What do you think?
@sandeep-krishnamurthy The code has been modified according to your requirements. Can you review it again? Thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the changes.
example/bayesian-methods/bdk_demo.py
Outdated
X_test=X_test, Y_test=Y_test, | ||
total_iter_num=1000000, | ||
initializer=initializer, | ||
learning_rate=4E-6, prior_precision=1.0, minibatch_size=100, | ||
thin_interval=100, burn_in_iter_num=1000) | ||
|
||
|
||
def run_mnist_DistilledSGLD(training_num=50000): | ||
def run_mnist_DistilledSGLD(training_num=50000, xpu=0): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This means, by default, mx.gpu(0). Should it default to CPU instead by making xpu=None?
@luobao-intel |
ping! @luobao-intel could you please take a look? |
@kalyc @ankkhedia Sorry for the delay. @luobao-intel is our summer intern and he is back to school now. |
@sandeep-krishnamurthy Could you take a look if your comments have been addressed with latest changes? Thanks. |
@vandanavk @sandeep-krishnamurthy I have change the code reference to the review comments, could you please make a review again? Thanks |
LGTM |
@sandeep-krishnamurthy could you take a look again? Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@vandanavk @roywei @sandeep-krishnamurthy Thanks for your reviews, and could this be merged? |
@sandeep-krishnamurthy could you help take a look again? Thanks. |
example/bayesian-methods/bdk_demo.py
Outdated
@@ -156,42 +156,42 @@ def get_toy_sym(teacher=True, teacher_noise_precision=None): | |||
return net | |||
|
|||
|
|||
def dev(): | |||
return mx.gpu() | |||
def dev(xpu = None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to have white spaces before and after =
. Also, xpu
is a little confusing for me as its value should be either gpu
or cpu
according to the name. But here in this case it equals to gpu numbers.
example/rcnn/symdata/loader.py
Outdated
@@ -129,7 +129,7 @@ def getpad(self): | |||
|
|||
class AnchorLoader(mx.io.DataIter): | |||
def __init__(self, roidb, batch_size, short, max_size, mean, std, | |||
feat_sym, anchor_generator: AnchorGenerator, anchor_sampler: AnchorSampler, | |||
feat_sym, anchor_generator=AnchorGenerator, anchor_sampler=AnchorSampler, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type of parameter anchor_generator must by Class AnchorGenerator, so i think it should be :
, not =
, i will do changes corresponding
example/rcnn/test.py
Outdated
@@ -94,7 +94,7 @@ def parse_args(): | |||
parser.add_argument('--params', type=str, default='', help='path to trained model') | |||
parser.add_argument('--dataset', type=str, default='voc', help='training dataset') | |||
parser.add_argument('--imageset', type=str, default='', help='imageset splits') | |||
parser.add_argument('--gpu', type=int, default=0, help='gpu device eg. 0') | |||
parser.add_argument('--gpu', type=int, default=0, help='gpu device eg. 0 if None then use cpu') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if not provide, then use cpu.
example/rcnn/train.py
Outdated
@@ -127,7 +127,7 @@ def parse_args(): | |||
parser.add_argument('--pretrained', type=str, default='', help='path to pretrained model') | |||
parser.add_argument('--dataset', type=str, default='voc', help='training dataset') | |||
parser.add_argument('--imageset', type=str, default='', help='imageset splits') | |||
parser.add_argument('--gpus', type=str, default='0', help='gpu devices eg. 0,1') | |||
parser.add_argument('--gpus', type=str, help='gpu devices eg. 0,1 if null then use cpu') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if not provide, then use cpu.
example/fcn-xs/fcn_xs.py
Outdated
@@ -85,6 +85,7 @@ def main(): | |||
help='the init type of fcn-xs model, e.g. vgg16, fcnxs') | |||
parser.add_argument('--retrain', action='store_true', default=False, | |||
help='true means continue training.') | |||
parser.add_argument("--gpu", type=int, help="if None then use cpu else use gpu") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
example/fcn-xs/fcn_xs.py
Outdated
|
||
def main(): | ||
ctx = mx.cpu() if not args.gpu else mx.gpu(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mx.gpu(args.gpu)? What's the value of args.gpu?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor comments. The rest looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems CI status is not reflected on the page. @pengxin99 Please re-trigger with an empty commit.
@sandeep-krishnamurthy Please check if your comments are addressed. |
Thank you for the contribution @pengxin99. I think it's good to merge now. |
* upstream/master: (109 commits) Code modification for testcases of various network models in directory example (apache#12498) [CI] Prevent timeouts when rebuilding containers with docker. (apache#13818) fix Makefile for rpkg (apache#13590) change to compile time (apache#13835) Disabled flaky test (apache#13758) Improve license_header tool by only traversing files under revision c… (apache#13803) Removes unneeded nvidia driver ppa installation (apache#13814) Add Local test stage and option to jump directly to menu item from commandline (apache#13809) Remove MXNET_STORAGE_FALLBACK_LOG_VERBOSE from test_autograd.py (apache#13830) Fix scala doc build break for v1.3.1 (apache#13820) [MXNET-1263] Unit Tests for Java Predictor and Object Detector APIs (apache#13794) [MXNET-1260] Float64 DType computation support in Scala/Java (apache#13678) onnx export ops (apache#13821) [MXNET-880] ONNX export: Random uniform, Random normal, MaxRoiPool (apache#13676) fix minor indentation (apache#13827) Fixing a symlink issue with R install (apache#13708) remove useless code (apache#13777) ONNX ops: norm exported and lpnormalization imported (apache#13806) Add new Maven build for Scala package (apache#13819) Dockerfiles for Publish Testing (apache#13707) ...
…y example (apache#12498) * example testcase modified * rcnn file add * license add * license init * CI test trigger * rcnn modify give up * trigger * modify for better user experience * change the default parameter to xpu=None * Update bdk_demo.py * Update fcn_xs.py * Update test.py * Update train.py * Update bdk_demo.py * Update bdk_demo.py * modify review comments * refine * modify Readmes according to the changed code. * finetune READMEs * re-trigger ci * re-trigger ci twice
Description
In the example directory, there are some test cases that exits some small bugs and missed options running on CPU. Thie pull request is to add the CPU options and fix some small bugs.
Checklist
Essentials
Please feel free to remove inapplicable items for your PR.
Changes
add CPU option
small bugs fix
file location change
Comments
@pengzhao-intel @TaoLv @juliusshufan