[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[IMP-dev] attribute table bug fix



Here is a simplification of the attribute tables which also fixes an assertion failure when there are few attributes around.

It adds a new file IMP_keys.i which defines some base classes for the Keys. This file should probably be somehow included in the other module's .is. I stuck it in IMPEM.i, but Ben needs to make sure such things are correct.

The patch includes the previous patches since they can't easily be separated at this point.
Index: em/pyext/IMPEM.i
===================================================================
--- em/pyext/IMPEM.i	(revision 669)
+++ em/pyext/IMPEM.i	(working copy)
@@ -7,6 +7,7 @@
 %}
 
 %include "kernel/pyext/IMP_macros.i"
+%include "kernel/pyext/IMP_keys.i"
 
 /* Ignore shared object import/export stuff */
 #define EMDLLEXPORT
Index: kernel/pyext/IMP.i
===================================================================
--- kernel/pyext/IMP.i	(revision 669)
+++ kernel/pyext/IMP.i	(working copy)
@@ -11,11 +11,21 @@
 
 %include "IMP_macros.i"
 %include "IMP_exceptions.i"
+%include "IMP_keys.i"
 
 /* Return derivatives from unary functions */
 %include "typemaps.i"
-%apply double &OUTPUT { IMP::Float& deriv };
 
+namespace IMP {
+  %typemap(out) std::pair<Float,Float> {
+     PyObject *tup= PyTuple_New(2);
+     PyTuple_SetItem(tup, 0, PyFloat_FromDouble($1.first));
+     PyTuple_SetItem(tup, 1, PyFloat_FromDouble($1.second));
+     $result= tup;
+  }
+}
+
+
 %pythoncode %{
 def check_particle(p, a):
    if (not p.get_is_active()):
@@ -27,94 +37,30 @@
 %}
 
 namespace IMP {
-  %pythonprepend Model::add_restraint %{
-        args[1].thisown=0
-  %}
-  %pythonprepend Model::add_score_state %{
-        args[1].thisown=0
-  %}
-  %pythonprepend Optimizer::add_optimizer_state %{
-        args[1].thisown=0
-  %}
-  %pythonprepend RestraintSet::add_restraint %{
-        args[1].thisown=0
-  %}
-  %pythonprepend NonbondedListScoreState::add_bonded_list %{
-        args[1].thisown=0
-  %}
-  %pythonprepend DistanceRestraint::DistanceRestraint %{
-        args[0].thisown=0
-  %}
-  %pythonprepend AngleRestraint::AngleRestraint %{
-        args[0].thisown=0
-  %}
-  %pythonprepend DihedralRestraint::DihedralRestraint %{
-        args[0].thisown=0
-  %}
-  %pythonprepend TorusRestraint::TorusRestraint %{
-        args[3].thisown=0
-  %}
-  %pythonprepend NonbondedRestraint::NonbondedRestraint %{
-        args[0].thisown=0
-  %}
-  %pythonprepend BondDecoratorRestraint::BondDecoratorRestraint %{
-        args[0].thisown=0
-  %}
-  %pythonprepend SingletonListRestraint::SingletonListRestraint %{
-        args[0].thisown=0
-  %}
-  %pythonprepend PairListRestraint::PairListRestraint %{
-        args[0].thisown=0
-  %}
-  %pythonprepend TripletChainRestraint::TripletChainRestraint %{
-        args[0].thisown=0
-  %}
-  %pythonprepend PairChainRestraint::PairChainRestraint %{
-        args[0].thisown=0
-  %}
-  %pythonprepend ConnectivityRestraint::ConnectivityRestraint %{
-        args[0].thisown=0
-  %}
-  %pythonprepend DistancePairScore::DistancePairScore %{
-        args[0].thisown=0
-  %}
-  %pythonprepend TransformedDistancePairScore::TransformedDistancePairScore %{
-        args[0].thisown=0
-  %}
-  %pythonprepend BondCoverPairScore::BondCoverPairScore %{
-        args[0].thisown=0
-  %}
-  %pythonprepend SphereDistancePairScore::SphereDistancePairScore %{
-        args[0].thisown=0
-  %}
-  %pythonprepend RefineOncePairScore::RefineOncePairScore %{
-        args[0].thisown=0
-        args[1].thisown=0
-  %}
-  %pythonprepend DistanceToSingletonScore::DistanceToSingletonScore %{
-        args[0].thisown=0
-  %}
-  %pythonprepend AttributeSingletonScore::AttributeSingletonScore %{
-        args[0].thisown=0
-  %}
-  %pythonprepend TunnelSingletonScore::TunnelSingletonScore %{
-        args[0].thisown=0
-  %}
-  %pythonprepend AngleTripletScore::AngleTripletScore %{
-        args[0].thisown=0
-  %}
-  %pythonprepend MonteCarlo::add_mover %{
-        args[1].thisown=0
-  %}
-  %pythonprepend MonteCarlo::set_local_optimizer %{
-        args[1].thisown=0
-  %}
-  %pythonprepend VRMLLogOptimizerState::add_particle_refiner %{
-        args[1].thisown=0
-  %}
-  %pythonprepend TypedPairScore::set_pair_score %{
-        args[1].thisown=0
-  %}
+  // need to special case particle so can't add this to macro
+  IMP_OWN_FIRST_CONSTRUCTOR(DistanceRestraint)
+  IMP_OWN_FIRST_CONSTRUCTOR(AngleRestraint)
+  IMP_OWN_FIRST_CONSTRUCTOR(DihedralRestraint)
+  IMP_OWN_FIRST_CONSTRUCTOR(TorusRestraint)
+  IMP_OWN_FIRST_CONSTRUCTOR(NonbondedRestraint)
+  IMP_OWN_FIRST_CONSTRUCTOR(BondDecoratorRestraint)
+  IMP_OWN_FIRST_CONSTRUCTOR(SingletonListRestraint)
+  IMP_OWN_FIRST_CONSTRUCTOR(PairListRestraint)
+  IMP_OWN_FIRST_CONSTRUCTOR(TripletChainRestraint)
+  IMP_OWN_FIRST_CONSTRUCTOR(PairChainRestraint)
+  IMP_OWN_FIRST_CONSTRUCTOR(ConnectivityRestraint)
+  IMP_OWN_FIRST_CONSTRUCTOR(DistancePairScore)
+  IMP_OWN_FIRST_CONSTRUCTOR(TransformedDistancePairScore)
+  IMP_OWN_FIRST_CONSTRUCTOR(BondCoverPairScore)
+  IMP_OWN_FIRST_CONSTRUCTOR(SphereDistancePairScore)
+  IMP_OWN_FIRST_SECOND_CONSTRUCTOR(RefineOncePairScore)
+  IMP_OWN_FIRST_CONSTRUCTOR(DistanceToSingletonScore)
+  IMP_OWN_FIRST_CONSTRUCTOR(AttributeSingletonScore)
+  IMP_OWN_FIRST_CONSTRUCTOR(TunnelSingletonScore)
+  IMP_OWN_FIRST_CONSTRUCTOR(AngleTripletScore)
+  IMP_SET_OBJECT(MonteCarlo, set_local_optimizer)
+  IMP_SET_OBJECT(TypedPairScore, set_pair_score)
+
   %pythonprepend Particle::get_value %{
         check_particle(args[0], args[1])
   %}
@@ -144,10 +90,19 @@
 
   %}
 
-  IMP_CONTAINER_SWIG(Model, Particle, particle);
+  // special case since particles are ref counted
+  %extend Model {
+    Particles get_particles() const {
+      IMP::Particles ret(self->particles_begin(), self->particles_end());
+      return ret;
+    }
+  }
   IMP_CONTAINER_SWIG(Model, ScoreState, score_state);
   IMP_CONTAINER_SWIG(Model, Restraint, restraint);
   IMP_CONTAINER_SWIG(RestraintSet, Restraint, restraint);
+  IMP_CONTAINER_SWIG(MonteCarlo, Mover, mover);
+  IMP_CONTAINER_SWIG(Optimizer, OptimizerState, optimizer_state);
+  IMP_CONTAINER_SWIG(NonbondedListScoreState, BondedListScoreState, bonded_list);
 }
 
 %feature("ref")   Particle "$this->ref();"
@@ -168,8 +123,10 @@
 %feature("director") IMP::TripletScore;
 %feature("director") IMP::Optimizer;
 %feature("director") IMP::ParticleRefiner;
+%feature("director") IMP::Mover;
 
-%include "IMP/Key.h"
+
+%include "IMP/base_types.h"
 %include "IMP/Object.h"
 %include "IMP/RefCountedObject.h"
 %include "IMP/Index.h"
@@ -259,12 +216,6 @@
   %template(OptimizerStateIndex) Index<OptimizerStateTag>;
   %template(MoverIndex) Index<Mover>;
   %template(BondedListIndex) Index<BondedListScoreState>;
-  %template(FloatKey) KeyBase<Float>;
-  %template(IntKey) KeyBase<Int>;
-  %template(StringKey) KeyBase<String>;
-  %template(ParticleKey) KeyBase<Particle*>;
-  %template(AtomType) KeyBase<AtomTypeTag>;
-  %template(ResidueType) KeyBase<ResidueTypeTag>;     
   %template(show_named_hierarchy) show<NameDecorator>;
   %template(show_molecular_hierarchy) show<MolecularHierarchyDecorator>;
   %template(Particles) ::std::vector<Particle*>;
Index: kernel/pyext/IMP_keys.i
===================================================================
--- kernel/pyext/IMP_keys.i	(revision 0)
+++ kernel/pyext/IMP_keys.i	(revision 0)
@@ -0,0 +1,11 @@
+%include "IMP/Key.h"
+
+namespace IMP
+{
+  %template(FloatKeyBase) ::IMP::KeyBase<0>;
+  %template(IntKeyBase) ::IMP::KeyBase<1>;
+  %template(StringKeyBase) ::IMP::KeyBase<2>;
+  %template(ParticleKeyBase) ::IMP::KeyBase<3>;
+  %template(AtomTypeBase) ::IMP::KeyBase<IMP_ATOM_TYPE_INDEX>;
+  %template(ResidueTypeBase) ::IMP::KeyBase<IMP_RESIDUE_TYPE_INDEX>;
+}
Index: kernel/src/Key.cpp
===================================================================
--- kernel/src/Key.cpp	(revision 669)
+++ kernel/src/Key.cpp	(working copy)
@@ -31,6 +31,9 @@
   }
 }
 
+
+std::map<unsigned int, KeyData> key_data;
+
 } // namespace internal
 
 } // namespace IMP
Index: kernel/src/decorators/ResidueDecorator.cpp
===================================================================
--- kernel/src/decorators/ResidueDecorator.cpp	(revision 669)
+++ kernel/src/decorators/ResidueDecorator.cpp	(working copy)
@@ -16,8 +16,6 @@
 namespace IMP
 {
 
-IMP_DEFINE_KEY_TYPE(ResidueType, ResidueTypeTag);
-
 IntKey ResidueDecorator::type_key_;
 IntKey ResidueDecorator::index_key_;
 
Index: kernel/src/decorators/AtomDecorator.cpp
===================================================================
--- kernel/src/decorators/AtomDecorator.cpp	(revision 669)
+++ kernel/src/decorators/AtomDecorator.cpp	(working copy)
@@ -15,8 +15,6 @@
 namespace IMP
 {
 
-IMP_DEFINE_KEY_TYPE(AtomType, AtomTypeTag);
-
 #define TYPE_INIT(STR) AT_##STR= AtomType(#STR);
 #define TYPE_INIT2(NAME, STR) AT_##NAME = AtomType(#STR);
 
Index: kernel/src/Particle.cpp
===================================================================
--- kernel/src/Particle.cpp	(revision 669)
+++ kernel/src/Particle.cpp	(working copy)
@@ -43,17 +43,15 @@
 
 void Particle::zero_derivatives()
 {
-  for (FloatKeyIterator it= float_keys_begin(); it != float_keys_end(); ++it) {
-    derivatives_.set_value(*it, 0);
-  }
+  derivatives_.set_values(0);
 }
 
 
 void Particle::show(std::ostream& out) const
 {
-  const char* inset = "  ";
+  const std::string inset("  ");
   out << std::endl;
-  out << "--" << get_index() << "--" << std::endl;
+  out << "Particle: " << get_index() << std::endl;
   if (is_active_) {
     out << inset << inset << "active";
   } else {
@@ -62,18 +60,24 @@
   out << std::endl;
 
   if (get_model() != NULL) {
-    out << inset << inset << "float attributes:" << std::endl;
-    floats_.show(out, "    ");
+    out << inset << "float attributes:" << std::endl;
+    floats_.show(out, inset+inset);
 
-    out << inset << inset << "int attributes:" << std::endl;
-    ints_.show(out, "    ");
+    out << inset << "float derivatives:" << std::endl;
+    derivatives_.show(out, inset+inset);
 
-    out << inset << inset << "string attributes:" << std::endl;
-    strings_.show(out, "    ");
+    out << inset << "optimizeds:" << std::endl;
+    optimizeds_.show(out, inset+inset);
 
-    out << inset << inset << "particle attributes:" << std::endl;
-    particles_.show(out, "    ");
+    out << inset << "int attributes:" << std::endl;
+    ints_.show(out, inset+inset);
 
+    out << inset << "string attributes:" << std::endl;
+    strings_.show(out, inset+inset);
+
+    out << inset << "particle attributes:" << std::endl;
+    particles_.show(out, inset+inset);
+
   }
 }
 
Index: kernel/include/IMP/utility.h
===================================================================
--- kernel/include/IMP/utility.h	(revision 669)
+++ kernel/include/IMP/utility.h	(working copy)
@@ -8,8 +8,16 @@
 #ifndef __IMP_UTILITY_H
 #define __IMP_UTILITY_H
 
+#include "base_types.h"
 #include "macros.h"
 
+#ifdef __GNUC__
+/** \todo Use an sconscript test for the isnan function directly
+ */
+#include <cmath>
+#endif
+
+
 namespace IMP
 {
 
@@ -20,6 +28,15 @@
   return t*t;
 }
 
+//! put in check for G++
+inline bool is_nan(Float a) {
+#ifdef __GNUC__
+  return std::isnan(a);
+#else
+  return a != a;
+#endif
+}
+
 } // namespace IMP
 
 #endif  /* __IMP_UTILITY_H */
Index: kernel/include/IMP/Particle.h
===================================================================
--- kernel/include/IMP/Particle.h	(revision 669)
+++ kernel/include/IMP/Particle.h	(working copy)
@@ -60,7 +60,7 @@
   struct ParticleAttributeTableTraits
   {
     typedef Pointer<Particle> Value;
-    typedef KeyBase<Particle*> Key;
+    typedef ParticleKey Key;
     static Value get_invalid() {
       return Value();
     }
@@ -69,13 +69,13 @@
     }
   };
 
- typedef internal::AttributeTable<internal::FloatAttributeTableTraits> 
+ typedef internal::AttributeTable<internal::FloatAttributeTableTraits>
    FloatTable;
- typedef internal::AttributeTable<internal::FloatAttributeTableTraits> 
+ typedef internal::AttributeTable<internal::FloatAttributeTableTraits>
    DerivativeTable;
- typedef internal::AttributeTable<internal::BoolAttributeTableTraits> 
+ typedef internal::AttributeTable<internal::BoolAttributeTableTraits>
    OptimizedTable;
- typedef internal::AttributeTable<internal::IntAttributeTableTraits>  
+ typedef internal::AttributeTable<internal::IntAttributeTableTraits>
    IntTable;
   typedef internal::AttributeTable<internal::StringAttributeTableTraits>
     StringTable;
Index: kernel/src/base_types.cpp
===================================================================
--- kernel/src/base_types.cpp	(revision 669)
+++ kernel/src/base_types.cpp	(working copy)
@@ -10,9 +10,4 @@
 namespace IMP
 {
 
-IMP_DEFINE_KEY_TYPE(FloatKey, Float);
-IMP_DEFINE_KEY_TYPE(IntKey, Int);
-IMP_DEFINE_KEY_TYPE(StringKey, String);
-IMP_DEFINE_KEY_TYPE(ParticleKey, Particle*)
-
 } // namespace IMP
Index: kernel/include/IMP/Key.h
===================================================================
--- kernel/include/IMP/Key.h	(revision 669)
+++ kernel/include/IMP/Key.h	(working copy)
@@ -21,13 +21,11 @@
 
    The keys in IMP maintain a cached mapping between strings and indexes.
    This mapping is global--that is all IMP Models and Particles in the
-   same program use the same mapping.
+   same program use the same mapping for each type of key. The type of
+   the key is determined by an integer which should be unique for
+   each type. If the integer is not unique, everything works, just
+   more memory is wasted and types are interconvertible
 
-   The mapping uses a static table which is defined with the
-   IMP_DEFINE_KEY_TYPE macro. As a result, the macro must be used in exactly
-   one .o. If it appears more than once or that .o is linked in multiple times,
-   then bad things can happen.
-
    Since the order of initialization of static data is undefined
    between .os, it is important that no other static data not in the
    same .o uses the key data. Specifically, typedes defined by
@@ -37,54 +35,43 @@
    indices and would make the set of indices less dense.
  */
 
-// Swig chokes on the specialization
-#ifndef SWIG
-//! \internal
-#define IMP_KEY_DATA_HOLDER(Name, Tag)                                  \
-  namespace internal {                                                  \
-  template <>                                                           \
-  struct IMPDLLEXPORT KeyDataHolder<Tag> {                              \
-    static KeyData data;                                                \
-  };                                                                    \
-  }
-#else
-#define IMP_KEY_DATA_HOLDER(Name, Tag)
-#endif
 
-
 /**
-   Define a new key type. There must be an accompanying IMP_DEFINE_KEY_TYPE
-   located in some .o file. This macro should be used in the IMP namespace.
+   Define a new key type.
 
    It defines two public types Name, which is an instantiation of KeyBase and
    Names which is a vector of Name.
 
    \param[in] Name The name for the new type.
-   \param[in] Tag A class which is unique for this type. For attributes
-   use the type of the attributes. For other Keys, declare an empty
-   class with a unique name and use it.
+   \param[in] Tag A (hopefully) unique integer to define this key type.
+
+   \note We define a new class rather than use a typedef since SWIG has a
+   bug dealing with names that start with ::. A fix has been commited to SVN
+   for swig.
+
+   \note The name in the typedef would have to start with ::IMP so it
+   could be used out of the IMP namespace.
  */
 #define IMP_DECLARE_KEY_TYPE(Name, Tag)                                 \
-  IMP_KEY_DATA_HOLDER(Name, Tag);                                       \
-  typedef KeyBase<Tag> Name;                                                \
-  typedef std::vector<Name> Name##s
+  struct Name: public ::IMP::KeyBase<Tag> {                             \
+  typedef ::IMP::KeyBase<Tag> P;                                        \
+  typedef Name This;                                                    \
+  Name(){};                                                             \
+  Name(unsigned int i): P(i){}                                          \
+  Name(const char *nm): P(nm){}                                         \
+};                                                                      \
+typedef std::vector<Name> Name##s
 
 
-/** This must occur in exactly one .o in the internal namespace. Should 
- be used in the IMP namespace.*/
-#define IMP_DEFINE_KEY_TYPE(Name, Tag)                  \
-  namespace internal {                                  \
-    KeyData KeyDataHolder<Tag>::data;                   \
-  }
-
-
 namespace IMP
 {
 
 namespace internal
 {
 
-/** The data concerning a particular type of key.
+
+
+/** The data concerning keys.
     \internal
  */
 struct IMPDLLEXPORT KeyData
@@ -109,16 +96,10 @@
   double heuristic_;
   Map map_;
   RMap rmap_;
-
 };
 
-/** A dummy class. Actual keys types create specializations
+IMPDLLEXPORT extern std::map<unsigned int, KeyData> key_data;
 
-    \internal
-*/
-template <class T>
-struct KeyDataHolder {};
-
 } // namespace internal
 
 
@@ -132,22 +113,45 @@
     attribute_table_index. Yes, this is an evil hack, but I couldn't
     get linking to work with static members of the template class.
  */
-template <class T>
+template <int ID>
 class KeyBase
 {
   int str_;
 
-  typedef T Type;
+  static const internal::KeyData::Map& get_map()
+  {
+    return IMP::internal::key_data[ID].get_map();
+  }
+  static const internal::KeyData::RMap& get_rmap() {
+    return IMP::internal::key_data[ID].get_rmap();
+  }
 
-  bool is_default() const;
 
-  static const internal::KeyData::Map& get_map();
-  static const internal::KeyData::RMap& get_rmap();
-
+  static unsigned int find_index(std::string sc) {
+    if (get_map().find(sc) == get_map().end()) {
+      return IMP::internal::key_data[ID].add_key(sc);
+    } else {
+      return get_map().find(sc)->second;
+    }
+  }
+protected:
+ bool is_default() const;
 public:
-  static const std::string &get_string(int i);
+  static const std::string get_string(int i)
+  {
+    if (static_cast<unsigned int>(i)
+        < get_rmap().size()) {
+      return get_rmap()[i];
+    } else {
+      IMP_WARN("Corrupted Key Table asking for key " << i
+                  << " with a table of size " << get_rmap().size());
+      std::ostringstream oss;
+      oss<< "Invalid index " << i;
+      return oss.str();
+    }
+  }
 
-  typedef KeyBase<T> This;
+  typedef KeyBase<ID> This;
 
   //! make a default (uninitalized) key
   KeyBase(): str_(-1) {}
@@ -155,16 +159,12 @@
 
   //! Generate a key from the given string
   KeyBase(const char *c) {
-    std::string sc(c);
-    if (get_map().find(sc) == get_map().end()) {
-      str_= internal::KeyDataHolder<T>::data.add_key(sc);
-    } else {
-      str_= get_map().find(sc)->second;
-    }    
+    str_= find_index(c);
   }
 
   explicit KeyBase(unsigned int i): str_(i) {
-    //IMP_assert(get_rmap().size() > i, "There is no such attribute " << i);
+    IMP_assert(str_ >= 0, "Invalid initializer " << i);
+    // cannot check here as we need a past end iterator
   }
 
   //! Turn a key into a pretty string
@@ -223,46 +223,28 @@
 #endif
 };
 
-IMP_OUTPUT_OPERATOR_1(KeyBase)
 
-
-template <class T>
-inline const internal::KeyData::Map& KeyBase<T>::get_map()
-{
-  return internal::KeyDataHolder<T>::data.get_map();
+template <int ID>
+std::ostream &operator<<(std::ostream &out, KeyBase<ID> k) {
+  k.show(out);
+  return out;
 }
 
-template <class T>
-inline const internal::KeyData::RMap& KeyBase<T>::get_rmap()
+template <int ID>
+inline bool KeyBase<ID>::is_default() const
 {
-  return internal::KeyDataHolder<T>::data.get_rmap();
-}
-
-template <class T>
-inline bool KeyBase<T>::is_default() const
-{
   return str_==-1;
 }
 
-template <class T>
-inline const std::string &KeyBase<T>::get_string(int i)
-{
-  IMP_assert(static_cast<unsigned int>(i)
-             < get_rmap().size(),
-             "Corrupted "  << " Key " << i
-             << " vs " << get_rmap().size());
-  return get_rmap()[i];
-}
 
-
-template <class T>
-inline void KeyBase<T>::show_all(std::ostream &out)
+template <int ID>
+inline void KeyBase<ID>::show_all(std::ostream &out)
 {
-  internal::KeyDataHolder<T>::data.show(out);
+  internal::key_data[ID].show(out);
 }
 
-template <class T>
-std::vector<std::string> KeyBase<T>::get_all_strings()
+template <int ID>
+std::vector<std::string> KeyBase<ID>::get_all_strings()
 {
   std::vector<std::string> str;
   for (internal::KeyData::Map::const_iterator it= get_map().begin();
Index: kernel/include/IMP/internal/AttributeTable.h
===================================================================
--- kernel/include/IMP/internal/AttributeTable.h	(revision 669)
+++ kernel/include/IMP/internal/AttributeTable.h	(working copy)
@@ -27,10 +27,14 @@
 namespace internal
 {
 
-struct FloatAttributeTableTraits
+template <class T, class K >
+struct DefaultTraits {
+  typedef T Value;
+  typedef K Key;
+};
+
+  struct FloatAttributeTableTraits: public DefaultTraits<Float, FloatKey>
 {
-  typedef Float Value;
-  typedef KeyBase<Float> Key;
   static Float get_invalid() {
     if (std::numeric_limits<Float>::has_quiet_NaN) {
       return std::numeric_limits<Float>::quiet_NaN();
@@ -42,17 +46,15 @@
   }
   static bool get_is_valid(Float f) {
     if (std::numeric_limits<Float>::has_quiet_NaN) {
-      return f==f;
+      return !is_nan(f);
     } else {
-      return f!= get_invalid();
+      return f != get_invalid();
     }
   }
 };
 
-struct IntAttributeTableTraits
+  struct IntAttributeTableTraits: public DefaultTraits<Int, IntKey>
 {
-  typedef Int Value;
-  typedef KeyBase<Int> Key;
   static Int get_invalid() {
     return std::numeric_limits<Int>::max();
   }
@@ -61,10 +63,8 @@
   }
 };
 
-struct BoolAttributeTableTraits
+struct BoolAttributeTableTraits: public DefaultTraits<bool, FloatKey >
 {
-  typedef bool Value;
-  typedef KeyBase<Float> Key;
   static bool get_invalid() {
     return false;
   }
@@ -73,10 +73,8 @@
   }
 };
 
-struct StringAttributeTableTraits
+  struct StringAttributeTableTraits: public DefaultTraits<String, StringKey>
 {
-  typedef String Value;
-  typedef KeyBase<String> Key;
   static Value get_invalid() {
     return "This is an invalid string in IMP";
   }
@@ -87,9 +85,9 @@
 
 // The traits for the particle class are declared in the Particle.h
 
-/** \internal 
+/** \internal
     If memory becomes a problem then either use a char table to look up
-    actual entries or use perfect hashing. 
+    actual entries or use perfect hashing.
     http://burtleburtle.net/bob/hash/perfect.html
 
     Actuall Cuckoo hashing is probably a better bet as that gets
@@ -105,42 +103,11 @@
 {
   typedef AttributeTable<Traits> This;
 
-  typedef boost::scoped_array<typename Traits::Value> Map; 
+  typedef std::vector<typename Traits::Value> Map;
   Map map_;
-  unsigned int size_;
 
-  typename Traits::Value* new_array(unsigned int asz) const {
-    typename Traits::Value* ret= new typename Traits::Value[asz];
-    for (unsigned int i=0; i< asz; ++i) {
-      ret[i]= Traits::get_invalid();
-    }
-    return ret;
-  }
-
-  void copy_from(unsigned int len, const Map &o) {
-    IMP_assert(map_, "Internal error in attribute table");
-    IMP_assert(size_ >= len, "Table too small");
-    //std::cout << "Copy from " << o << " to " << map_ << std::endl;
-    for (unsigned int i=0; i< len; ++i) {
-      map_[i]= o[i];
-    }
-  }
-
-  void realloc(unsigned int olen, const Map &o, unsigned int nlen) {
-    //std::cout << "Realloc from " << size_ << " " << map_ << " ";
-    if (nlen==0) {
-      size_=0;
-      map_.reset();
-    } else {
-      size_=std::max(nlen, 6U);
-      map_.reset(new_array(size_));
-      copy_from(olen, o);
-    }
-    //std::cout << " to " << size_ << " " << map_ << std::endl;
-  }
-
   void check_contains(typename Traits::Key k) const {
-    IMP_assert(size_ > k.get_index(),
+    IMP_assert(map_.size() > k.get_index(),
                "Attribute \"" << k.get_string()
                << "\" not found in table.");
     IMP_check(Traits::get_is_valid(map_[k.get_index()]),
@@ -149,26 +116,18 @@
               IndexException);
   }
 
+  void clear() {
+    map_.clear();
+  }
+
 public:
   typedef typename Traits::Value Value;
   typedef typename Traits::Key Key;
-  AttributeTable(): size_(0){}
-  AttributeTable(const This &o): size_(0) {
-    //std::cout << "Copy constructor called from " << o.map_ << std::endl;
-    realloc(o.size_, o.map_, o.size_);
-  }
+  AttributeTable(){}
   ~AttributeTable() {
-    //std::cout << "Deleting " << map_ << std::endl; 
+    //std::cout << "Deleting " << map_ << std::endl;
   }
-  This &operator=(const This &o) {
-    //std::cout << "Operator= called from " << o.map_ << std::endl;
-    if (&o == this) {
-      //std::cout << "Self assignment" << std::endl;
-      return *this;
-    }
-    realloc(o.size_, o.map_, o.size_);
-    return *this;
-  }
+
   const Value get_value(Key k) const {
     check_contains(k);
     return map_[k.get_index()];
@@ -183,6 +142,11 @@
     map_[k.get_index()] = v;
   }
 
+  void set_values(Value v) {
+    for (unsigned int i=0; i< map_.size(); ++i) {
+      map_[i]=v;
+    }
+  }
 
   void insert(Key k, Value v) {
     IMP_assert(!contains(k),
@@ -195,24 +159,27 @@
 
   void remove(Key k) {
     check_contains(k);
-    map_[k.get_index()]= Traits::get_invalid();
+    remove_always(k);
   }
 
   void remove_always(Key k) {
-    if (k.get_index() < size_) {
+    IMP_assert(k != Key(), "Can't remove invalid key");
+    if (k.get_index() < map_.size()) {
       map_[k.get_index()]= Traits::get_invalid();
+      while (!map_.empty()
+             && map_.back()== Traits::get_invalid()) map_.pop_back();
     }
   }
 
 
   bool contains(Key k) const {
     IMP_assert(k != Key(), "Can't search for default key");
-    return k.get_index() < size_
+    return k.get_index() < map_.size()
       && Traits::get_is_valid(map_[k.get_index()]);
   }
 
 
-  void show(std::ostream &out, const char *prefix="") const;
+  void show(std::ostream &out, const std::string prefix="") const;
 
 
   std::vector<Key> get_keys() const;
@@ -228,19 +195,26 @@
     }
   };
 
-  typedef boost::counting_iterator<Key, boost::random_access_traversal_tag,
-                                   std::size_t> KeyIterator;
+  typedef boost::counting_iterator<Key, boost::forward_traversal_tag,
+                                   unsigned int> KeyIterator;
   typedef boost::filter_iterator<IsAttribute, KeyIterator> AttributeKeyIterator;
 
   AttributeKeyIterator attribute_keys_begin() const {
+    KeyIterator b(0U);
+    KeyIterator e(map_.size());
+    IMP_assert(std::distance(b,e)
+               == map_.size(), "Something is broken with the iterators");
+    IMP_assert(std::distance(AttributeKeyIterator(IsAttribute(this), b,e),
+                             AttributeKeyIterator(IsAttribute(this), e,e))
+                             <= map_.size(), "Broken in filter");
     return AttributeKeyIterator(IsAttribute(this),
                                 KeyIterator(Key(0U)),
-                                KeyIterator(Key(size_)));
+                                KeyIterator(Key(map_.size())));
   }
   AttributeKeyIterator attribute_keys_end() const {
     return AttributeKeyIterator(IsAttribute(this),
-                                KeyIterator(Key(size_)),
-                                KeyIterator(Key(size_)));
+                                KeyIterator(Key(map_.size())),
+                                KeyIterator(Key(map_.size())));
   }
 
 };
@@ -253,19 +227,20 @@
 template <class Traits>
 inline void AttributeTable<Traits>::insert_always(Key k, Value v)
 {
-  /*std::cout << "Insert " << k << " in v of size " 
+  /*std::cout << "Insert " << k << " in v of size "
     << size_ << " " << map_ << " " << k.get_index() << std::endl;*/
   IMP_assert(k != Key(),
             "Can't insert default key");
   IMP_check(Traits::get_is_valid(v),
-            "Trying to insert invalid value for attribute " 
+            "Trying to insert invalid value for attribute "
             << v << " into attribute " << k,
             ValueException);
-  if (size_ <= k.get_index()) {
-    boost::scoped_array<Value> old;
-    swap(old, map_);
-    realloc(size_, old, k.get_index()+1);
-  }
+  typename Map::size_type val
+    =static_cast<typename Map::size_type>(k.get_index());
+  IMP_assert(val <1000, "Bad key index");
+  map_.resize(std::max(map_.size(),
+                       val+1),
+              Traits::get_invalid());
   map_[k.get_index()]= v;
 }
 
@@ -273,9 +248,9 @@
 
 template <class Traits>
 inline void AttributeTable<Traits>::show(std::ostream &out,
-                                        const char *prefix) const
+                                         const std::string prefix) const
 {
-  for (unsigned int i=0; i< size_; ++i) {
+  for (unsigned int i=0; i< map_.size(); ++i) {
     if (Traits::get_is_valid(map_[i])) {
       out << prefix;
       out << Key(i).get_string() << ": ";
@@ -287,12 +262,12 @@
 
 
 template <class Traits>
-inline std::vector<typename Traits::Key> 
+inline std::vector<typename Traits::Key>
   AttributeTable<Traits>::get_keys() const
 {
   std::vector<Key> ret(attribute_keys_begin(), attribute_keys_end());
   return ret;
-} 
+}
 
 
 } // namespace internal
Index: kernel/include/IMP/decorators/macros.h
===================================================================
--- kernel/include/IMP/decorators/macros.h	(revision 669)
+++ kernel/include/IMP/decorators/macros.h	(working copy)
@@ -262,4 +262,7 @@
   name##_data_.initialize();
 
 
+#define IMP_ATOM_TYPE_INDEX 8974343
+#define IMP_RESIDUE_TYPE_INDEX 90784334
+
 #endif  /* __IMP_DECORATOR_MACROS_H */
Index: kernel/include/IMP/decorators/ResidueDecorator.h
===================================================================
--- kernel/include/IMP/decorators/ResidueDecorator.h	(revision 669)
+++ kernel/include/IMP/decorators/ResidueDecorator.h	(working copy)
@@ -17,8 +17,7 @@
 namespace IMP
 {
 
-struct ResidueTypeTag{};
-IMP_DECLARE_KEY_TYPE(ResidueType, ResidueTypeTag);
+IMP_DECLARE_KEY_TYPE(ResidueType, IMP_RESIDUE_TYPE_INDEX);
 
 
 //! A decorator for a residue.
Index: kernel/include/IMP/decorators/AtomDecorator.h
===================================================================
--- kernel/include/IMP/decorators/AtomDecorator.h	(revision 669)
+++ kernel/include/IMP/decorators/AtomDecorator.h	(working copy)
@@ -13,6 +13,7 @@
 #include "../Model.h"
 #include "utility.h"
 #include "XYZDecorator.h"
+#include "macros.h"
 
 #include <vector>
 #include <deque>
@@ -20,8 +21,7 @@
 namespace IMP
 {
 
-struct AtomTypeTag{};
-IMP_DECLARE_KEY_TYPE(AtomType, AtomTypeTag);
+IMP_DECLARE_KEY_TYPE(AtomType, IMP_ATOM_TYPE_INDEX);
 
 
 //! A decorator for a particle representing an atom.
Index: kernel/include/IMP/base_types.h
===================================================================
--- kernel/include/IMP/base_types.h	(revision 669)
+++ kernel/include/IMP/base_types.h	(working copy)
@@ -14,7 +14,6 @@
 
 #include <string>
 #include <vector>
-#include <map>
 
 namespace IMP
 {
@@ -63,7 +62,7 @@
 class Particle;
 //! A class which is used for representing collections of particles
 /**
-   We need this to have a uniform return type for python. 
+   We need this to have a uniform return type for python.
    \todo It would be nice to use internal::Vector instead, but that
    is not as pretty for Python.
  */
@@ -77,13 +76,13 @@
 class Particle;
 
 //! The type used to identify float attributes in the Particles
-IMP_DECLARE_KEY_TYPE(FloatKey, Float);
+IMP_DECLARE_KEY_TYPE(FloatKey, 0);
 //! The type used to identify int attributes in the Particles
-IMP_DECLARE_KEY_TYPE(IntKey, Int);
+IMP_DECLARE_KEY_TYPE(IntKey, 1);
 //! The type used to identify string attributes in the Particles
-IMP_DECLARE_KEY_TYPE(StringKey, String);
+IMP_DECLARE_KEY_TYPE(StringKey, 2);
 //! The type used to identify a particle attribute in the Particles
-IMP_DECLARE_KEY_TYPE(ParticleKey, Particle*);
+IMP_DECLARE_KEY_TYPE(ParticleKey, 3);
 
 } // namespace IMP