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

Relaxed Tree #679

Merged
merged 41 commits into from Aug 7, 2012
Merged

Relaxed Tree #679

merged 41 commits into from Aug 7, 2012

Conversation

pluskid
Copy link
Contributor

@pluskid pluskid commented Jul 30, 2012

Hi all,

This is the Relaxed Tree Implementation in shogun. I think I finally get it done, please review the code and get it merged. Thanks!

@@ -4,7 +4,7 @@ VALGRINDOPTS=--tool=memcheck --error-limit=no --trace-children=yes --leak-check=

INCLUDES=-I/usr/include/atlas
LIBS=
LIBS_ADD=-lshogun
LIBS_ADD=-lshogun -lpthread
Copy link
Member

Choose a reason for hiding this comment

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

why is this needed? if shogun uses pthread related stuff in its header files the correct fix would be to move this code into the corresponding .cpp file

Copy link
Member

Choose a reason for hiding this comment

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

actually this will cause build errors. I will remove that - please fix your code to work w/o -lpthread 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.

Oh, sorry. I remember I fixed this, no idea why it is still there.

Copy link
Member

Choose a reason for hiding this comment

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

Seems like this was only still visible in the comments but not the code.

Chiyuan Zhang notifications@github.com wrote:

@@ -4,7 +4,7 @@ VALGRINDOPTS=--tool=memcheck --error-limit=no
--trace-children=yes --leak-check=

INCLUDES=-I/usr/include/atlas
LIBS=
-LIBS_ADD=-lshogun
+LIBS_ADD=-lshogun -lpthread

Oh, sorry. I remember I fixed this, no idea why it is still there.


Reply to this email directly or view it on GitHub:
https://github.com/shogun-toolbox/shogun/pull/679/files#r1328760

Sent from Kaiten Mail. Please excuse my brevity.

@pluskid
Copy link
Contributor Author

pluskid commented Aug 1, 2012

Hi @sonney2k, Thanks for so many comments. I've fixed most of them, and for those not fixed, I've provided response to your comments. Please review again.

@pluskid
Copy link
Contributor Author

pluskid commented Aug 6, 2012

Hi, I switched to using shallow_copy for copying CKernels.

/** A shalow copy.
* All the SGObject instance variables will be simply assigned and SG_REF-ed.
*/
virtual CSGObject *shalow_copy() const
Copy link
Member

Choose a reason for hiding this comment

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

typo: should be shallow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, I'm sorry for the typo. This is fixed now.

sonney2k pushed a commit that referenced this pull request Aug 7, 2012
@sonney2k sonney2k merged commit d5bbf37 into shogun-toolbox:master Aug 7, 2012
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

3 participants