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

[WIP]Refactor MulticlassMachine #5101

Conversation

LiuYuHui
Copy link
Contributor

@LiuYuHui LiuYuHui commented Jul 23, 2020

Fix by shogun-toolbox/shogun-libtooling#2

  • fix C++ code
  • fix unit test
  • fix meta example
  • fix jupyter notebook

@gf712
Copy link
Member

gf712 commented Jul 23, 2020

I guess this just has to be cleaned up a bit manually. And then can see if the compiler still throws errors, in which case there might be a class missing

@@ -64,6 +64,8 @@ class SVM : public KernelMachine
*/
SVM(float64_t C, std::shared_ptr<Kernel> k, std::shared_ptr<Labels> lab);
Copy link
Member

Choose a reason for hiding this comment

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

do you still need this constructor?

/* changing the subset structure to use subset stacks. This might
* have to be revised. Heiko Strathmann */
m_features->remove_subset();

Copy link
Member

Choose a reason for hiding this comment

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

if these methods becomes noops can they just be removed? How are the subsets handled now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just add/remove subset to feature that will be passed in train_machine.

Copy link
Member

Choose a reason for hiding this comment

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

I see, makes sense to me :)

@LiuYuHui LiuYuHui force-pushed the refacter_multiclass_machine branch from ed3e315 to aa02ae7 Compare July 25, 2020 10:31
@LiuYuHui LiuYuHui force-pushed the refacter_multiclass_machine branch from aa02ae7 to ef95d18 Compare July 25, 2020 11:30
@gf712 gf712 merged commit e2673fc into shogun-toolbox:feature/machine_refactor Jul 28, 2020
gf712 pushed a commit that referenced this pull request Jul 28, 2020
* Refactor Multiclass Machine
gf712 pushed a commit that referenced this pull request Dec 8, 2020
* Refactor Multiclass Machine
gf712 pushed a commit that referenced this pull request Dec 8, 2020
* Refactor Multiclass Machine
gf712 pushed a commit that referenced this pull request Dec 8, 2020
* Refactor Multiclass Machine
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