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

Fix inconsistency that could cause the optimization algorithm to oscillate #228

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 51 additions & 18 deletions svm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,37 @@ void Cache::swap_index(int i, int j)
}
}

// Provides access to the elements in a single column of a QMatrix.
class QColumn {
public:
QColumn(const Qfloat *values, int column_idx, double diagonal_value)
: values(values), column_idx(column_idx), diagonal_value(diagonal_value) {
}

double operator[](int row_idx) const {
if (row_idx != column_idx) {
return values[row_idx];
} else {
// Return the value from the QD array so that calculations stay
// consistent, regardless of whether QColumn or the QD array
// are used.
//
// QD array is double precision while Qfloat could be single
// precision, so the diagonal values could be very different
// between the two. If one calculation uses QColumn while
// another calculation uses the QD array, the optimization
// algorithm may not converge.
return diagonal_value;
}
}

private:
const Qfloat *values;

int column_idx;
double diagonal_value;
};

//
// Kernel evaluation
//
Expand All @@ -197,7 +228,7 @@ void Cache::swap_index(int i, int j)
//
class QMatrix {
public:
virtual Qfloat *get_Q(int column, int len) const = 0;
virtual QColumn get_Q(int column, int len) const = 0;
virtual double *get_QD() const = 0;
virtual void swap_index(int i, int j) const = 0;
virtual ~QMatrix() {}
Expand All @@ -210,7 +241,7 @@ class Kernel: public QMatrix {

static double k_function(const svm_node *x, const svm_node *y,
const svm_parameter& param);
virtual Qfloat *get_Q(int column, int len) const = 0;
virtual QColumn get_Q(int column, int len) const = 0;
virtual double *get_QD() const = 0;
virtual void swap_index(int i, int j) const // no so const...
{
Expand Down Expand Up @@ -486,7 +517,7 @@ void Solver::reconstruct_gradient()
{
for(i=active_size;i<l;i++)
{
const Qfloat *Q_i = Q->get_Q(i,active_size);
QColumn Q_i = Q->get_Q(i,active_size);
for(j=0;j<active_size;j++)
if(is_free(j))
G[i] += alpha[j] * Q_i[j];
Expand All @@ -497,7 +528,7 @@ void Solver::reconstruct_gradient()
for(i=0;i<active_size;i++)
if(is_free(i))
{
const Qfloat *Q_i = Q->get_Q(i,l);
QColumn Q_i = Q->get_Q(i,l);
double alpha_i = alpha[i];
for(j=active_size;j<l;j++)
G[j] += alpha_i * Q_i[j];
Expand Down Expand Up @@ -548,7 +579,7 @@ void Solver::Solve(int l, const QMatrix& Q, const double *p_, const schar *y_,
for(i=0;i<l;i++)
if(!is_lower_bound(i))
{
const Qfloat *Q_i = Q.get_Q(i,l);
QColumn Q_i = Q.get_Q(i,l);
double alpha_i = alpha[i];
int j;
for(j=0;j<l;j++)
Expand Down Expand Up @@ -594,8 +625,8 @@ void Solver::Solve(int l, const QMatrix& Q, const double *p_, const schar *y_,

// update alpha[i] and alpha[j], handle bounds carefully

const Qfloat *Q_i = Q.get_Q(i,active_size);
const Qfloat *Q_j = Q.get_Q(j,active_size);
QColumn Q_i = Q.get_Q(i,active_size);
QColumn Q_j = Q.get_Q(j,active_size);

double C_i = get_C(i);
double C_j = get_C(j);
Expand Down Expand Up @@ -822,9 +853,11 @@ int Solver::select_working_set(int &out_i, int &out_j)
}

int i = Gmax_idx;
const Qfloat *Q_i = NULL;
if(i != -1) // NULL Q_i not accessed: Gmax=-INF if i=-1
Q_i = Q->get_Q(i,active_size);
if (i == -1) {
return 1;
}

QColumn Q_i = Q->get_Q(i,active_size);

for(int j=0;j<active_size;j++)
{
Expand Down Expand Up @@ -1071,8 +1104,8 @@ int Solver_NU::select_working_set(int &out_i, int &out_j)

int ip = Gmaxp_idx;
int in = Gmaxn_idx;
const Qfloat *Q_ip = NULL;
const Qfloat *Q_in = NULL;
QColumn Q_ip = QColumn(NULL, -1, 0);
QColumn Q_in = QColumn(NULL, -1, 0);
if(ip != -1) // NULL Q_ip not accessed: Gmaxp=-INF if ip=-1
Q_ip = Q->get_Q(ip,active_size);
if(in != -1)
Expand Down Expand Up @@ -1280,7 +1313,7 @@ class SVC_Q: public Kernel
QD[i] = (this->*kernel_function)(i,i);
}

Qfloat *get_Q(int i, int len) const
QColumn get_Q(int i, int len) const
{
Qfloat *data;
int start, j;
Expand All @@ -1292,7 +1325,7 @@ class SVC_Q: public Kernel
for(j=start;j<len;j++)
data[j] = (Qfloat)(y[i]*y[j]*(this->*kernel_function)(i,j));
}
return data;
return QColumn(data, i, QD[i]);
}

double *get_QD() const
Expand Down Expand Up @@ -1332,7 +1365,7 @@ class ONE_CLASS_Q: public Kernel
QD[i] = (this->*kernel_function)(i,i);
}

Qfloat *get_Q(int i, int len) const
QColumn get_Q(int i, int len) const
{
Qfloat *data;
int start, j;
Expand All @@ -1341,7 +1374,7 @@ class ONE_CLASS_Q: public Kernel
for(j=start;j<len;j++)
data[j] = (Qfloat)(this->*kernel_function)(i,j);
}
return data;
return QColumn(data, i, QD[i]);
}

double *get_QD() const
Expand Down Expand Up @@ -1398,7 +1431,7 @@ class SVR_Q: public Kernel
swap(QD[i],QD[j]);
}

Qfloat *get_Q(int i, int len) const
QColumn get_Q(int i, int len) const
{
Qfloat *data;
int j, real_i = index[i];
Expand All @@ -1417,7 +1450,7 @@ class SVR_Q: public Kernel
schar si = sign[i];
for(j=0;j<len;j++)
buf[j] = (Qfloat) si * (Qfloat) sign[j] * data[index[j]];
return buf;
return QColumn(buf, i, QD[i]);
}

double *get_QD() const
Expand Down