9
\$\begingroup\$

Here is a simple Hopfield network that I wrote with the help of a book that I am reading. I would like you to review it and give me your suggestions, especially for the node-updating part (the last part of the main function).

Example:

The network should remember these patterns: 1111, 1010, 0101, 1001, 0110.

If you enter:

1100

The network will recall this pattern:

0110

Hopfield.h

#include <stdio.h>
#include <iostream>
#include <math.h>

class Hopfield_neuron
   {
     protected:
            int activation;
            friend class Hopfield_network;

     public:
            int weight[4];
            Hopfield_neuron() {};
            Hopfield_neuron(int *j);
            int act(int , int*);
   };

  class Hopfield_network
   {
     public:
     Hopfield_neuron neuron[4];
     int output[4];
     int threshold(int);
     void update(int, int*, int*);
     void activation(int j[4]);
     Hopfield_network(int*, int*, int*, int*);
   };

main.cpp

#include "Hopfield.h";

Hopfield_neuron::Hopfield_neuron(int *j)
{
     for(int i=0; i<4; i++)
       {
           weight[i] = *(j+i);
       }
}

int Hopfield_neuron::act(int m, int *pattern)
{
    int a = 0;

    for(int i=0; i<4; i++)
      {
         a += pattern[i] * weight[i];
      }

     return a;
 }

 int Hopfield_network::threshold(int k)
 {
     if ( k >= 0)
         return 1;
     else
        return 0;
  }

 void Hopfield_network::update(int node, int *pattern, int *weight)
 {
        int vin = 0;
        for(int i=0; i<4; i++)
          {
             vin += pattern[i] * weight[i];
          }

         if(threshold(vin) != pattern[node])
           {
                pattern[node] = threshold(vin);
           }
  }

 Hopfield_network::Hopfield_network(int nrn0[4], int nrn1[4], int nrn2[4],        int nrn3[4])
  {
          neuron[0] = Hopfield_neuron(nrn0);
          neuron[1] = Hopfield_neuron(nrn1);
          neuron[2] = Hopfield_neuron(nrn2);
          neuron[3] = Hopfield_neuron(nrn3);
  }

  void Hopfield_network::activation(int *pattern)
  {
         for(int i=0; i<4; i++)
           {
              neuron[i].activation = neuron[i].act(4, pattern);
              output[i] = threshold(neuron[i].activation);
           }
  } 

  void main()
  {
         int pattern1[] = {1,1,0,0};

         int weight1[] = { 0, -5,  4,  4};
         int weight2[] = {-5,  0,  4,  4};
         int weight3[] = { 4,  4,  0, -5};
         int weight4[] = { 4,  4, -5,  0};

        std::cout<<"This is hopfield network with a single layer of 4";
        std::cout<<"\nfully interconnected neurons. The network should  recall the \nPatterns 1111, 1010, 0101, 1001, 0110 correctly.\n";

        Hopfield_network h1(weight1, weight2, weight3, weight4);

        h1.activation(pattern1);

       for(int i=0; i<4; i++)
          {
              if(h1.output[i] == pattern1[i])
               {
                   std::cout<<"\n pattern = "<<pattern1[i]<<"  output = "<<h1.output[i]<<"  component matches";
               }
              else
              {
                   std::cout<<"\n pattern = "<<pattern1[i]<<"  output = "<<h1.output[i]<<"  discrepancy occured";
              }
         }


        int choosedNodes[8] = {1,2,3,4,1,2,3,4};
        int weight[4];

        for(int i=0; i<8; i++)
          {
              switch(choosedNodes[i])
                 {
                    case 1: weight[0] =  0; weight[1] = -5;  weight[2] =   4;  weight[3] =  4; 
                    break;
                    case 2: weight[0] = -5; weight[1] =  0;  weight[2] =   4;  weight[3] =  4; 
                    break;
                    case 3: weight[0] =  4; weight[1] =  4;  weight[2] =   0;  weight[3] = -5;  
                    break;
                    case 4: weight[0] =  4; weight[1] = 4;   weight[2]  = -5;  weight[3] =  0; 
                     break;

                  }
             h1.update(choosedNodes[i]-1, pattern1, weight);
         }

         std::cout<<"\n\n The recalled Pattern\n";


         for(int i=0; i<4; i++)
           {
               std::cout<<"\n pattern = "<<pattern1[i];

           }

          std::cin.get();
  }
\$\endgroup\$
1
  • \$\begingroup\$ Shouldn't you be passing in 12 weight attribute with 4 neuorns? Since a hopfield network cannot have a connection to itself, so 4*3 = 12. \$\endgroup\$ Commented Oct 15, 2018 at 5:56

1 Answer 1

9
\$\begingroup\$

Your indentation is very inconsistent. Pick a number of spaces for indentation and stick with it.

Hopfield.h

  • You're using some C libraries (<stdio.h> and <math.h>) in C++. They should respectively be <cstdio> and <cmath>.

    You may also remove both of them as they don't appear to be in use.

    Lastly, you should move <iostream> to the .cpp file since it's not used in the header. In general, header files should have as few links as possible because other files dependent on them and may be affected by them and break.

  • In Hopfield_neuron, it looks like you should have private instead of protected. The latter would only be needed if another class were to inherit from it. Also, weight should not be public as it's a data member.

  • Hopfield_network shouldn't be entirely public. The first three data members should be private while the member functions can stay public.

main.cpp

  • Do not use void as main()'s return type. It may be allowed by some non-compliant compilers, but it's still considered non-standard. It should only return int.

  • You use the "magic number" 4 for your array bounds. You could consider making it a constant so that its meaning is given. This would also allow you to make changes in just one area if you ever need to modify this value.

  • threshold() can just use a single-line ternary statement:

    return (k >= 0) ? 1 : 0;
    

    You should also make it const since it doesn't modify any data members:

    int Hopfield_network::threshold(int k) const {}
    
  • Remove that excess whitespace from the end of the Hopfield_network() parameter line:

    Hopfield_network::Hopfield_network(int nrn0[4], int nrn1[4], int nrn2[4],        int nrn3[4])
    
  • Use storage containers such as std::vector in place of C-style arrays. The latter should not be used in C++ if at all possible. One reason is because passing them to functions causes them to decay to pointers. This is already happening in many of your functions.

  • Consider putting each statement in the switch as separate lines. This could help with readability and with preventing horizontal line character counts from increasing.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.