CLabels::is_valid should only be called at proper situation.

It is not proper to call this in constructor. For example, the return value from apply() might be an "invalid" BinaryLabels because not both positive and negative labels presented.
shogun-toolbox · May 22, 2012 · 91de8f9 · 91de8f9
1 parent 4d35f91
commit 91de8f9
Show file tree

Hide file tree

Showing 10 changed files with 44 additions and 47 deletions.
diff --git a/src/shogun/labels/BinaryLabels.cpp b/src/shogun/labels/BinaryLabels.cpp
@@ -35,9 +35,9 @@ CBinaryLabels* CBinaryLabels::obtain_from_generic(CLabels* base_labels)
 }
 
 
-bool CBinaryLabels::is_valid()
-{       
-    ASSERT(m_labels.vector);
+void CBinaryLabels::ensure_valid(const char* context)
+{
+    CDenseLabels::ensure_valid(context);
     bool found_plus_one=false;
     bool found_minus_one=false;
 
@@ -51,17 +51,22 @@ bool CBinaryLabels::is_valid()
             found_minus_one=true;
         else
         {
-            SG_ERROR("Not a two class labeling label[%d]=%f (only +1/-1 "
-                    "allowed)\n", i, m_labels[real_i]);
+            SG_ERROR("%s%sNot a two class labeling label[%d]=%f (only +1/-1 "
+                    "allowed)\n", context?context:"", context?": ":"", i, m_labels[real_i]);
         }
     }
 
     if (!found_plus_one)
-        SG_ERROR("Not a two class labeling - no positively labeled examples found\n");
-    if (!found_minus_one)
-        SG_ERROR("Not a two class labeling - no negatively labeled examples found\n");
+    {
+        SG_ERROR("%s%sNot a two class labeling - no positively labeled examples found\n",
+                context?context:"", context?": ":"");
+    }
 
-    return true;
+    if (!found_minus_one)
+    {
+        SG_ERROR("%s%sNot a two class labeling - no negatively labeled examples found\n",
+                context?context:"", context?": ":"");
+    }
 }
 
 ELabelType CBinaryLabels::get_label_type()

diff --git a/src/shogun/labels/BinaryLabels.h b/src/shogun/labels/BinaryLabels.h
@@ -60,13 +60,13 @@ class CBinaryLabels : public CDenseLabels
 		 */
 		static CBinaryLabels* obtain_from_generic(CLabels* base_labels);
 
-		/** is_valid checks if labeling is a two-class labeling
+		/** Make sure the label is valid, otherwise raise SG_ERROR.
 		 *
 		 * possible with subset
-		 *
-		 * @return if this is two-class labeling
+         *
+         * @param context optional message to convey the context
 		 */
-		virtual bool is_valid();
+		virtual void ensure_valid(const char* context=NULL);
 
 		/** get label type
 		 *

diff --git a/src/shogun/labels/DenseLabels.cpp b/src/shogun/labels/DenseLabels.cpp
@@ -73,8 +73,6 @@ void CDenseLabels::set_labels(SGVector<float64_t> v)
 		SG_ERROR("A subset is set, cannot set labels\n");
 
 	m_labels = v;
-
-	is_valid();
 }
 
 SGVector<float64_t> CDenseLabels::get_labels()
@@ -121,6 +119,12 @@ void CDenseLabels::set_int_labels(SGVector<int32_t> lab)
 		set_int_label(i, lab.vector[i]);
 }
 
+void CDenseLabels::ensure_valid(const char* context)
+{
+    if (m_labels.vector == NULL)
+        SG_ERROR("%s%sempty content (NULL) for labels\n", context?context:"", context?": ":"");
+}
+
 void CDenseLabels::load(CFile* loader)
 {
 	remove_subset();

diff --git a/src/shogun/labels/DenseLabels.h b/src/shogun/labels/DenseLabels.h
@@ -51,13 +51,13 @@ class CDenseLabels : public CLabels
 		/** destructor */
 		virtual ~CDenseLabels();
 
-		/** check if labeling is valid 
+		/** Make sure the label is valid, otherwise raise SG_ERROR.
 		 *
 		 * possible with subset
-		 *
-		 * @return if labeling is valid (e.g. binary labeling)
+         *
+         * @param context optional message to convey the context
 		 */
-		virtual bool is_valid()=0;
+		virtual void ensure_valid(const char* context=NULL);
 
 		/** load labels from file
 		 *

diff --git a/src/shogun/labels/Labels.h b/src/shogun/labels/Labels.h
@@ -40,13 +40,13 @@ class CLabels : public CSGObject
 		/** destructor */
 		virtual ~CLabels();
 
-		/** check if labeling is valid 
+		/** Make sure the label is valid, otherwise raise SG_ERROR.
 		 *
 		 * possible with subset
-		 *
-		 * @return if labeling is valid (e.g. binary labeling)
+         *
+         * @param context optional message to convey the context
 		 */
-		virtual bool is_valid()=0;
+		virtual void ensure_valid(const char* context=NULL)=0;
 
 		/** get number of labels, depending on whether a subset is set
 		 *

diff --git a/src/shogun/labels/MulticlassLabels.cpp b/src/shogun/labels/MulticlassLabels.cpp
@@ -31,9 +31,9 @@ CMulticlassLabels* CMulticlassLabels::obtain_from_generic(CLabels* base_labels)
 	return NULL;
 }
 
-bool CMulticlassLabels::is_valid()
+void CMulticlassLabels::ensure_valid(const char* context)
 {       
-    ASSERT(m_labels.vector);
+    CDenseLabels::ensure_valid(context);
 
     int32_t subset_size=get_num_labels();
     for (int32_t i=0; i<subset_size; i++)
@@ -43,12 +43,10 @@ bool CMulticlassLabels::is_valid()
 
         if (label<0 || float64_t(label)!=m_labels[real_i])
 		{
-			SG_ERROR("Multiclass Labels must be in range 0...<nr_classes-1> and integers!\n");
-			return false;
+			SG_ERROR("%s%sMulticlass Labels must be in range 0...<nr_classes-1> and integers!\n",
+                    context?context:"", context?": ":"");
 		}
 	}
-
-    return true;
 }
 
 ELabelType CMulticlassLabels::get_label_type()

diff --git a/src/shogun/labels/MulticlassLabels.h b/src/shogun/labels/MulticlassLabels.h
@@ -60,13 +60,13 @@ class CMulticlassLabels : public CDenseLabels
 		 */
 		static CMulticlassLabels* obtain_from_generic(CLabels* base_labels);
 
-		/** is_valid checks if labeling is a multi-class labeling
+		/** Make sure the label is valid, otherwise raise SG_ERROR.
 		 *
 		 * possible with subset
-		 *
-		 * @return if this is multi-class labeling
+         *
+         * @param context optional message to convey the context
 		 */
-		virtual bool is_valid();
+		virtual void ensure_valid(const char* context=NULL);
 
 		/** get label type
 		 *

diff --git a/src/shogun/labels/RegressionLabels.cpp b/src/shogun/labels/RegressionLabels.cpp
@@ -30,12 +30,6 @@ CRegressionLabels* CRegressionLabels::obtain_from_generic(CLabels* base_labels)
 	return NULL;
 }
 
-bool CRegressionLabels::is_valid()
-{       
-	ASSERT(m_labels.vector);
-	return true;
-}
-
 ELabelType CRegressionLabels::get_label_type()
 {
 	return LT_REGRESSION;

diff --git a/src/shogun/labels/RegressionLabels.h b/src/shogun/labels/RegressionLabels.h
@@ -61,14 +61,6 @@ class CRegressionLabels : public CDenseLabels
 		 */
 		static CRegressionLabels* obtain_from_generic(CLabels* base_labels);
 
-		/** is_valid checks if labeling is a multi-class labeling
-		 *
-		 * possible with subset
-		 *
-		 * @return if this is multi-class labeling
-		 */
-		virtual bool is_valid();
-
 		/** get label type
 		 *
 		 * @return label type real

diff --git a/src/shogun/machine/Machine.cpp b/src/shogun/machine/Machine.cpp
@@ -56,6 +56,10 @@ bool CMachine::train(CFeatures* data)
 				get_name());
 	}
 
+    if (m_labels == NULL)
+        SG_ERROR("%s@%p: No labels given", get_name(), this);
+    m_labels->ensure_valid(get_name());
+
 	bool result = train_machine(data);
 
 	if (m_store_model_features)