New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[GSOC]DatasetMapper & Imputer #694
Changes from 1 commit
87c05a5
2e4b1a8
631e59e
391006e
6a1fb81
b0c5224
de35241
2d38604
bb045b8
1295f4b
5a517c2
94b7a5c
ebed68f
db78f39
7c60b97
da4e409
d8618ec
3b8ffd0
90a5cd2
32c8a73
e09d9bc
87d8d46
de0b2db
bc187ca
a340f69
21d94c0
a92afaa
bace8b2
896a018
1a908c2
2edbc40
d881cb7
a881831
63268a3
2eb6754
6d43aa3
fedc5e0
787fd82
a0b7d59
9a6dce7
c3aeba1
028c217
03e19a4
ef4536b
6e2c1ff
5eb9abd
d043235
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,7 +96,11 @@ bool Load(const std::string& filename, | |
arma::Mat<eT>& matrix, | ||
DatasetMapper<PolicyType>& info, | ||
const bool fatal = false, | ||
const bool transpose = true); | ||
const bool transpose = true) | ||
{ | ||
PolicyType policy; | ||
return Load(filename, matrix, info, policy, fatal, transpose); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since you already provide an api to access the policy of the DatasetMapper, I think we could remove another Load function which allow user to pass the policy. This would make the api more consistent. You can create the DatasetMapper in the Load function without the policy parameters pass
By this way users only need to store the state in their DatasetMapper only, I think this is less confusing.Else the users may think If you want to allow the users to get/set their policy states, you can add two api to the DatasetMapper
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks for the feedback! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think it is a good idea |
||
} | ||
|
||
/** | ||
* Loads a matrix from a file, guessing the filetype from the extension and | ||
|
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.
I think the return type should be PolicyType const&, else the compiler may able to compile(this make sense, const member function return reference non-const data member is weird).
Try to compile following codes, you will find it cannot compile