Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

DType regression #3018

Merged
merged 6 commits into from
Aug 16, 2016
Merged

DType regression #3018

merged 6 commits into from
Aug 16, 2016

Conversation

Godricly
Copy link
Contributor

  • DType regression
  • Python test for DType pooling and regression

Tensor<xpu, 2, DType> grad = in_grad[reg_enum::kData].FlatTo2D<xpu, DType>(s);
Tensor<xpu, 2, DType> label = in_data[reg_enum::kLabel]
.get_with_shape<xpu, 2, DType>(out.shape_, s);
Assign(grad, req[reg_enum::kData], scalar<DType>(param_.grad_scale/num_output)*
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this correct? What if Dtype is int8?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your review. Which line are you worry about? I think int8 is currently not supported as we are using MSHADOW_REAL_TYPE_SWITCH to create operators.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, merge as is for now. We have to change a lot for the coming unit8...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep... I'll propose a pr to fix nullptr issue in other operators later. Is there any paper list to share about uint8 network?

There are some other issues to be fixed to make DType network really works, like kvstore, param init. For the lstm case, extra states data_type is need. We need to discuss more about this DType support and a more detail todo list.

@winstywang
Copy link
Contributor

Some inline comments are added. After fixing them, it is good to merge in

@winstywang
Copy link
Contributor

Please update to latest master, so that I can merge

@Godricly
Copy link
Contributor Author

Cpp test failed after the update.

@winstywang winstywang merged commit 8cc2560 into apache:master Aug 16, 2016
@Godricly Godricly deleted the DTypeRegression_commit branch August 16, 2016 16:35
@winstywang
Copy link
Contributor

@Godricly We have to revert this PR until you provide correct test scripts and corresponding results.

@Godricly Godricly restored the DTypeRegression_commit branch August 22, 2016 17:55
@Godricly
Copy link
Contributor Author

Sry for that typo. I don't have GPU access now, so a fix will take some time. The operator should be ok. Does that script get called in travis?

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.

None yet

2 participants