Skip to content
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

Refactor all machine #5104

Merged

Conversation

LiuYuHui
Copy link
Contributor

@LiuYuHui LiuYuHui commented Aug 3, 2020

No description provided.

@LiuYuHui LiuYuHui changed the title [WIP] Refacter all machine [WIP] Refactor all machine Aug 4, 2020
"Number of training vectors ({}) does not match number of "
"labels ({})",
data->get_num_vectors(), m_labels->get_num_labels());

Copy link
Member

Choose a reason for hiding this comment

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

these checks are not needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

move these checks to NonParametricMachine::train

src/shogun/machine/Pipeline.h Outdated Show resolved Hide resolved
src/shogun/machine/Pipeline.h Outdated Show resolved Hide resolved
@gf712
Copy link
Member

gf712 commented Aug 6, 2020

Cool! I guess once this is merged we can start the PR against the develop branch :)

@@ -50,7 +50,8 @@ TEST_F(PipelineTest, fit_predict)
->then(machine);

// no labels given
EXPECT_THROW(pipeline->train(features), ShogunException);
//EXPECT_THROW(pipeline->train(features), ShogunException);
//pipeline->train(features, labels);
Copy link
Member

Choose a reason for hiding this comment

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

what happens here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

now pipeline have two train method, original train will not throw a exception.

@LiuYuHui LiuYuHui changed the title [WIP] Refactor all machine Refactor all machine Aug 7, 2020
@gf712
Copy link
Member

gf712 commented Aug 10, 2020

@LiuYuHui Can I merge this?

@LiuYuHui
Copy link
Contributor Author

@LiuYuHui Can I merge this?

@gf712 yes :D

@gf712 gf712 merged commit 5794acb into shogun-toolbox:feature/machine_refactor Aug 10, 2020
gf712 pushed a commit that referenced this pull request Dec 8, 2020
* refactor all machines
* fix unit test
* fix python legacy and jupyter notebook
gf712 pushed a commit that referenced this pull request Dec 8, 2020
* refactor all machines
* fix unit test
* fix python legacy and jupyter notebook
gf712 pushed a commit that referenced this pull request Dec 8, 2020
* refactor all machines
* fix unit test
* fix python legacy and jupyter notebook
gf712 pushed a commit that referenced this pull request Dec 8, 2020
* refactor all machines
* fix unit test
* fix python legacy and jupyter notebook
gf712 pushed a commit that referenced this pull request Dec 8, 2020
* refactor all machines
* fix unit test
* fix python legacy and jupyter notebook
gf712 pushed a commit that referenced this pull request Dec 8, 2020
* refactor all machines
* fix unit test
* fix python legacy and jupyter notebook
gf712 pushed a commit that referenced this pull request Dec 8, 2020
* refactor all machines
* fix unit test
* fix python legacy and jupyter notebook
gf712 pushed a commit that referenced this pull request Dec 8, 2020
* refactor all machines
* fix unit test
* fix python legacy and jupyter notebook
gf712 pushed a commit that referenced this pull request Dec 8, 2020
* refactor all machines
* fix unit test
* fix python legacy and jupyter notebook
gf712 pushed a commit that referenced this pull request Dec 8, 2020
* refactor all machines
* fix unit test
* fix python legacy and jupyter notebook
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants