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

[PrematureStopping] Add to CMachine premature stopping methods. #3858

Merged
merged 6 commits into from Jun 25, 2017

Conversation

Projects
None yet
2 participants
@geektoni
Contributor

geektoni commented Jun 23, 2017

This will need a rebase when #3853 will get merged.

[PrematureStopping] Register SIGINT signal handler.
The signal handler is set up when calling init_shogun(). It is
disabled by default (sg_signal->enable_handler() is needed).
Applied also style fixes.
@@ -39,6 +41,7 @@ namespace shogun
Version* sg_version=NULL;
CMath* sg_math=NULL;
CRandom* sg_rand=NULL;
CSignal* sg_signal = NULL;

This comment has been minimized.

@vigsterkr

vigsterkr Jun 23, 2017

Member

we could make this one a std::unique_ptr<CSignal> sg_signal(nullptr) since we are c++11 and not c++98

This comment has been minimized.

@vigsterkr

vigsterkr Jun 23, 2017

Member

or Some<CSignal>? :)

@@ -191,12 +201,20 @@ namespace shogun
SG_REF(sg_rand);
return sg_rand;
}
CSignal* get_global_signal()

This comment has been minimized.

@vigsterkr

vigsterkr Jun 23, 2017

Member

this as well could be part of #ifndef SWIG?
do we wanna expose signals to swig interfaces? @karlnapf @lisitsyn

void (*print_message)(FILE* target, const char* str) = NULL,
void (*print_warning)(FILE* target, const char* str) = NULL,
void (*print_error)(FILE* target, const char* str) = NULL,
void (*cancel_computations)(bool& delayed, bool& immediately) = NULL);

This comment has been minimized.

@vigsterkr

vigsterkr Jun 23, 2017

Member

miju.... caramba :) std::function plz

if (i == SG_PAUSE_COMP)
this->on_pause();
else if (i == SG_BLOCK_COMP)
this->on_next();

This comment has been minimized.

@vigsterkr

vigsterkr Jun 23, 2017

Member

what if else?

@@ -374,13 +374,13 @@ TEST(LeastAngleRegression, cholesky_insert)
SGMatrix<float64_t> R(num_feats, num_feats);
SGMatrix<float64_t> mat(num_vec, num_feats-1);
SGMatrix<float64_t> matnew(num_vec, num_feats);

This comment has been minimized.

@vigsterkr

vigsterkr Jun 23, 2017

Member

why does this mostly whitespace changes have to do with this patch?

@vigsterkr

This comment has been minimized.

Member

vigsterkr commented Jun 23, 2017

@geektoni ok sorry i did here the review of both commits :S

geektoni added some commits Jun 23, 2017

[PrematureStopping] Add on_next(), on_pause() and on_complete() metho…
…d to CMachine.

The default behaviour of these methods is to set one of two variable called
m_cancel_computation or m_pause_computation. They can be used to terminate prematurely
algorithms or to pause them, by pressing CTRL+C.

Fixed also a bug inside lars_unittest.cc
[PrematureStopping] Use unique_ptr to wrap CSignal instance.
Replace also function pointers with std::function.
@@ -54,6 +59,7 @@ bool CMachine::train(CFeatures* data)
m_labels->ensure_valid(get_name());
}
connect_to_signal_handler();
bool result = train_machine(data);

This comment has been minimized.

@vigsterkr

vigsterkr Jun 23, 2017

Member

once a machine is trained (prematurely stopped or not), shouldn't we disconnect here and reset the cancel_computation state so if one wants to re-train the same machine could still do it?

@@ -306,6 +315,37 @@ class CMachine : public CSGObject
return PT_BINARY;
}
#ifndef SWIG

This comment has been minimized.

@vigsterkr

vigsterkr Jun 24, 2017

Member

formatting... clang-format failed :)

}
#endif
#ifndef SWIG

This comment has been minimized.

@vigsterkr

vigsterkr Jun 24, 2017

Member

formatting... clang-format failed :)

{
return m_cancel_computation.load();
}
#endif

This comment has been minimized.

@vigsterkr

vigsterkr Jun 24, 2017

Member

formatting... clang-format failed :)

m_pause_computation.wait(lck);
}
}
#endif

This comment has been minimized.

@vigsterkr

vigsterkr Jun 24, 2017

Member

formatting... clang-format failed :)

}
#endif
#ifndef SWIG

This comment has been minimized.

@vigsterkr

vigsterkr Jun 24, 2017

Member

formatting... clang-format failed :)

#ifndef SWIG
/** Resume current computation (sets the flag) */
SG_FORCED_INLINE void resume_computation()

This comment has been minimized.

@vigsterkr

vigsterkr Jun 24, 2017

Member

formatting... clang-format failed :)

m_pause_computation_flag = false;
m_pause_computation.notify_all();
}
#endif

This comment has been minimized.

@vigsterkr

vigsterkr Jun 24, 2017

Member

formatting... clang-format failed :)

[PrematureStopping] Improve pause method using std::condition_variabl…
…e and locks.

Add COMPUTATION_CONTROLLERS macro to manage pause/cancel.
It will works also inside OpenMP environment.

Guarded from SWIG some methods which use SG_FORCED_INLINE.

@vigsterkr vigsterkr merged commit 10bb3ba into shogun-toolbox:feature/premature-stopping Jun 25, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
default DEV build done.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment