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.

Sign up for GitHub

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

Jump to bottom

Alternative fix for Dynamic(Object)Array::clone() #3721

Merged
merged 3 commits into from Mar 23, 2017
Merged

Alternative fix for Dynamic(Object)Array::clone() #3721

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2d532dd
Add unit test for CDynamicObjectArray::clone()
Mar 20, 2017
0ce0ce3
Remove "name" feature in CDynamicArray, CDynamicObjectArray
Mar 20, 2017
cd3df66
Fix CDynamicArray and CDynamicObjectArray serialization
Mar 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 0 additions & 18 deletions src/shogun/base/SGObject.cpp
View file
Open in desktop
Expand Up @@ -711,24 +711,6 @@ bool CSGObject::equals(CSGObject* other, float64_t accuracy, bool tolerant)
SG_DEBUG("comparing parameter \"%s\" to other's \"%s\"\n",
this_param->m_name, other_param->m_name);

/* hard-wired exception for DynamicObjectArray parameter num_elements */
Copy link
Member

Choose a reason for hiding this comment

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

aaaah, so much nicer :)

if (!strcmp("DynamicObjectArray", get_name()) &&
!strcmp(this_param->m_name, "num_elements") &&
!strcmp(other_param->m_name, "num_elements"))
{
SG_DEBUG("Ignoring DynamicObjectArray::num_elements field\n");
continue;
}

/* hard-wired exception for DynamicArray parameter num_elements */
if (!strcmp("DynamicArray", get_name()) &&
!strcmp(this_param->m_name, "num_elements") &&
!strcmp(other_param->m_name, "num_elements"))
{
SG_DEBUG("Ignoring DynamicArray::num_elements field\n");
continue;
}

/* use equals method of TParameter from here */
if (!this_param->equals(other_param, accuracy, tolerant))
{
Expand Down
46 changes: 19 additions & 27 deletions src/shogun/lib/DynamicArray.h
View file
Open in desktop
Expand Up @@ -32,7 +32,7 @@ template <class T> class CDynamicArray :public CSGObject
public:
/** default constructor */
CDynamicArray()
: CSGObject(), m_array(), name("Array")
: CSGObject(), m_array()
{
dim1_size=1;
dim2_size=1;
Expand All @@ -48,7 +48,7 @@ template <class T> class CDynamicArray :public CSGObject
* @param p_dim3_size dimension 3
*/
CDynamicArray(int32_t p_dim1_size, int32_t p_dim2_size=1, int32_t p_dim3_size=1)
: CSGObject(), m_array(p_dim1_size*p_dim2_size*p_dim3_size), name("Array")
: CSGObject(), m_array(p_dim1_size*p_dim2_size*p_dim3_size)
{
dim1_size=p_dim1_size;
dim2_size=p_dim2_size;
Expand All @@ -65,7 +65,7 @@ template <class T> class CDynamicArray :public CSGObject
* @param p_copy_array if array must be copied
*/
CDynamicArray(T* p_array, int32_t p_dim1_size, bool p_free_array, bool p_copy_array)
: CSGObject(), m_array(p_array, p_dim1_size, p_free_array, p_copy_array), name("Array")
: CSGObject(), m_array(p_array, p_dim1_size, p_free_array, p_copy_array)
{
dim1_size=p_dim1_size;
dim2_size=1;
Expand All @@ -84,7 +84,7 @@ template <class T> class CDynamicArray :public CSGObject
*/
CDynamicArray(T* p_array, int32_t p_dim1_size, int32_t p_dim2_size,
bool p_free_array, bool p_copy_array)
: CSGObject(), m_array(p_array, p_dim1_size*p_dim2_size, p_free_array, p_copy_array), name("Array")
: CSGObject(), m_array(p_array, p_dim1_size*p_dim2_size, p_free_array, p_copy_array)
{
dim1_size=p_dim1_size;
dim2_size=p_dim2_size;
Expand All @@ -104,7 +104,7 @@ template <class T> class CDynamicArray :public CSGObject
*/
CDynamicArray(T* p_array, int32_t p_dim1_size, int32_t p_dim2_size,
int32_t p_dim3_size, bool p_free_array, bool p_copy_array)
: CSGObject(), m_array(p_array, p_dim1_size*p_dim2_size*p_dim3_size, p_free_array, p_copy_array), name("Array")
: CSGObject(), m_array(p_array, p_dim1_size*p_dim2_size*p_dim3_size, p_free_array, p_copy_array)
{
dim1_size=p_dim1_size;
dim2_size=p_dim2_size;
Expand All @@ -121,7 +121,7 @@ template <class T> class CDynamicArray :public CSGObject
* @param p_dim3_size dimension 3
*/
CDynamicArray(const T* p_array, int32_t p_dim1_size=1, int32_t p_dim2_size=1, int32_t p_dim3_size=1)
: CSGObject(), m_array(p_array, p_dim1_size*p_dim2_size*p_dim3_size), name("Array")
: CSGObject(), m_array(p_array, p_dim1_size*p_dim2_size*p_dim3_size)
{
dim1_size=p_dim1_size;
dim2_size=p_dim2_size;
Expand Down Expand Up @@ -545,21 +545,6 @@ template <class T> class CDynamicArray :public CSGObject
/** shuffles the array with external random state */
inline void shuffle(CRandom * rand) { m_array.shuffle(rand); }

/** set array's name
*
* @param p_name new name
*/
inline void set_array_name(const char* p_name)
{
name=p_name;
}

/** get array's name
*
* @return array's name
*/
inline const char* get_array_name() const { return name; }

/** display this array */
inline void display_array()
{
Expand Down Expand Up @@ -619,6 +604,16 @@ template <class T> class CDynamicArray :public CSGObject
m_array.resize_array(m_array.get_num_elements(), true);
}

virtual CSGObject* clone()
{
CDynamicArray * cloned = (CDynamicArray*) CSGObject::clone();
// Since the array vector is registered with
// current_num_elements as size (see parameter
// registration) the cloned version has less memory
// allocated than known to dynarray. We fix this here.
cloned->m_array.num_elements = cloned->m_array.current_num_elements;
Copy link
Member

Choose a reason for hiding this comment

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

So again:

  • we don't register the size of allocated memory parameter
  • serializing/deserializing will change it. Storing only stores the used part of the array, loading only allocated as much as needed, and re-sizes when something is added next time
  • it is not part of equals as it is not registered

return cloned;
}

private:

Expand All @@ -630,9 +625,6 @@ template <class T> class CDynamicArray :public CSGObject
m_parameters->add_vector(&m_array.array,
&m_array.current_num_elements, "array",
"Memory for dynamic array.");
SG_ADD(&m_array.num_elements,
"num_elements",
"Number of Elements.", MS_NOT_AVAILABLE);
SG_ADD(&m_array.resize_granularity,
"resize_granularity",
"shrink/grow step size.", MS_NOT_AVAILABLE);
Expand All @@ -644,6 +636,9 @@ template <class T> class CDynamicArray :public CSGObject
"free_array",
"whether array must be freed",
MS_NOT_AVAILABLE);
SG_ADD(&dim1_size, "dim1_size", "Dimension 1", MS_NOT_AVAILABLE);
SG_ADD(&dim2_size, "dim2_size", "Dimension 2", MS_NOT_AVAILABLE);
SG_ADD(&dim3_size, "dim3_size", "Dimension 3", MS_NOT_AVAILABLE);
}

protected:
Expand All @@ -659,9 +654,6 @@ template <class T> class CDynamicArray :public CSGObject

/** dimension 3 */
int32_t dim3_size;

/** array's name */
const char* name;
};
}
#endif /* _DYNAMIC_ARRAY_H_ */
47 changes: 19 additions & 28 deletions src/shogun/lib/DynamicObjectArray.h
View file
Open in desktop
Expand Up @@ -33,7 +33,7 @@ class CDynamicObjectArray : public CSGObject
public:
/** default constructor */
CDynamicObjectArray()
: CSGObject(), m_array(), name("Array")
: CSGObject(), m_array()
{
dim1_size=1;
dim2_size=1;
Expand All @@ -49,7 +49,7 @@ class CDynamicObjectArray : public CSGObject
* @param dim3 dimension 3
*/
CDynamicObjectArray(int32_t dim1, int32_t dim2=1, int32_t dim3=1)
: CSGObject(), m_array(dim1*dim2*dim3), name("Array")
: CSGObject(), m_array(dim1*dim2*dim3)
{
dim1_size=dim1;
dim2_size=dim2;
Expand All @@ -66,7 +66,7 @@ class CDynamicObjectArray : public CSGObject
* @param p_copy_array if array must be copied
*/
CDynamicObjectArray(CSGObject** p_array, int32_t p_dim1_size, bool p_free_array=true, bool p_copy_array=false)
: CSGObject(), m_array(p_array, p_dim1_size, p_free_array, p_copy_array), name("Array")
: CSGObject(), m_array(p_array, p_dim1_size, p_free_array, p_copy_array)
{
dim1_size=p_dim1_size;
dim2_size=1;
Expand All @@ -85,7 +85,7 @@ class CDynamicObjectArray : public CSGObject
*/
CDynamicObjectArray(CSGObject** p_array, int32_t p_dim1_size, int32_t p_dim2_size,
bool p_free_array=true, bool p_copy_array=false)
: CSGObject(), m_array(p_array, p_dim1_size*p_dim2_size, p_free_array, p_copy_array), name("Array")
: CSGObject(), m_array(p_array, p_dim1_size*p_dim2_size, p_free_array, p_copy_array)
{
dim1_size=p_dim1_size;
dim2_size=p_dim2_size;
Expand All @@ -105,7 +105,7 @@ class CDynamicObjectArray : public CSGObject
*/
CDynamicObjectArray(CSGObject** p_array, int32_t p_dim1_size, int32_t p_dim2_size,
int32_t p_dim3_size, bool p_free_array=true, bool p_copy_array=false)
: CSGObject(), m_array(p_array, p_dim1_size*p_dim2_size*p_dim3_size, p_free_array, p_copy_array), name("Array")
: CSGObject(), m_array(p_array, p_dim1_size*p_dim2_size*p_dim3_size, p_free_array, p_copy_array)
{
dim1_size=p_dim1_size;
dim2_size=p_dim2_size;
Expand Down Expand Up @@ -393,21 +393,6 @@ class CDynamicObjectArray : public CSGObject
/** shuffles the array with external random state */
inline void shuffle(CRandom * rand) { m_array.shuffle(rand); }

/** set array's name
*
* @param p_name new name
*/
inline void set_array_name(const char* p_name)
{
name=p_name;
}

/** get array's name
*
* @return array's name
*/
inline const char* get_array_name() const { return name; }

/** @return object name */
virtual const char* get_name() const
{ return "DynamicObjectArray"; }
Expand Down Expand Up @@ -442,16 +427,23 @@ class CDynamicObjectArray : public CSGObject
m_array.resize_array(m_array.get_num_elements(), true);
}

private:
virtual CSGObject* clone()
{
CDynamicObjectArray* cloned = (CDynamicObjectArray*) CSGObject::clone();
// Since the array vector is registered with
// current_num_elements as size (see parameter
// registration) the cloned version has less memory
// allocated than known to dynarray. We fix this here.
cloned->m_array.num_elements = cloned->m_array.current_num_elements;
return cloned;
}

private:
/** register parameters */
virtual void init()
{
m_parameters->add_vector(&m_array.array, &m_array.current_num_elements, "array",
"Memory for dynamic array.");
SG_ADD(&m_array.num_elements,
"num_elements",
"Number of Elements.", MS_NOT_AVAILABLE);
SG_ADD(&m_array.resize_granularity,
"resize_granularity",
"shrink/grow step size.", MS_NOT_AVAILABLE);
Expand All @@ -463,6 +455,9 @@ class CDynamicObjectArray : public CSGObject
"free_array",
"whether array must be freed",
MS_NOT_AVAILABLE);
SG_ADD(&dim1_size, "dim1_size", "Dimension 1", MS_NOT_AVAILABLE);
SG_ADD(&dim2_size, "dim2_size", "Dimension 2", MS_NOT_AVAILABLE);
SG_ADD(&dim3_size, "dim3_size", "Dimension 3", MS_NOT_AVAILABLE);
}

/** de-reference all elements of this array once */
Expand All @@ -487,10 +482,6 @@ class CDynamicObjectArray : public CSGObject

/** dimension 3 */
int32_t dim3_size;

/** array's name */
const char* name;

};
}
#endif /* _DYNAMIC_OBJECT_ARRAY_H_ */