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

factories are factories #4604

Merged
merged 1 commit into from Apr 16, 2019
Merged

Conversation

vigsterkr
Copy link
Member

they are always gonna love :)

@vigsterkr
Copy link
Member Author

this is an attempt to fix #4321

@gf712
Copy link
Member

gf712 commented Apr 10, 2019

erf, seems like it isn't that straightforward...

@gf712
Copy link
Member

gf712 commented Apr 10, 2019

create_object in class_list.h also increments the ref count

@vigsterkr
Copy link
Member Author

imo that refcount++ inn create_object should be dropped... but i'm gonna test it on donbot

@vigsterkr
Copy link
Member Author

vigsterkr commented Apr 12, 2019

this became a memory leaking cleanup pr

@@ -21,8 +21,8 @@ combined_kernel.add("kernel_array", k_3)
#![create_kernels]

#![create_classifier]
SVM libsvm = as_svm(machine("LibSVM"))
Machine svm = machine("MKLClassification", kernel=combined_kernel, interleaved_optimization=False, svm=libsvm)
SVM libsvm = machine("LibSVM")
Copy link
Member Author

Choose a reason for hiding this comment

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

for some reason if as_svm is being used here the ref counting is broken :S

@@ -25,8 +25,8 @@ combined_kernel.init(features_train, features_train)
#![create_combined_train]

#![train_mkl]
SVM binary_svm_solver = as_svm(machine("SVRLight"))
Machine mkl = machine("MKLRegression", svm=binary_svm_solver, kernel=combined_kernel, labels=labels_train)
SVM binary_svm_solver = machine("SVRLight")
Copy link
Member Author

Choose a reason for hiding this comment

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

for some reason if as_svm is being used here the ref counting is broken :S

Copy link
Member

Choose a reason for hiding this comment

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

broken in which way? too high or too low?

@@ -1,3 +1,28 @@
%newobject shogun::distance(std::string name);
Copy link
Member

Choose a reason for hiding this comment

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

doesn't it matter that it is const std::string&?

@vigsterkr vigsterkr force-pushed the factory_swig branch 3 times, most recently from 1aa6cdf to 8b6d3ed Compare April 16, 2019 06:05
they are always gonna love :)
@vigsterkr vigsterkr merged commit afac518 into shogun-toolbox:develop Apr 16, 2019
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.

None yet

2 participants