PanPalitta / phase_estimation

This project apply reinforcement learning algorithms based on DE and PSO to optimize adaptive quantum-phase estimation.
http://panpalitta.github.io/phase_estimation/
GNU General Public License v3.0
9 stars 5 forks source link

Reorganizing main #32

Closed peterwittek closed 8 years ago

peterwittek commented 8 years ago

Once #26 is completed, we can simplify and restructure main.cpp. The primary changes should be:

  1. Move initialization to OptAlg class.
  2. Move accept-reject criterion to OptAlg class.
  3. Linear interpolation routines should be move to the auxiliary functions.
  4. After this, a culling would be welcome on the remaining variables. Also, some variable names should be changed to something more expressive.
peterwittek commented 8 years ago

Since in the main loop the iteration variable could change backwards, it should not be a for but a while loop instead.

PanPalitta commented 8 years ago

The main function is now at least partly cleaned up. The accept-reject criteria and linear interpolation routines are hidden, but still needs more work to generalize the accept-reject criteria for other problems. Variables that are now not used in the main function are deleted. However, some work still needs to be done on:

  1. The initialization; I look at it and was not sure how to move it as it depends on numvar
  2. Include preprocessor directives in main and phase_loss_opt to allow change in loss rate at the compiling stage.
  3. Generalize accept-reject criteria; I don't have an elegant solution to this yet, but I can see including the policy_check into OptAlg::check_success() and instead of changing numvar, we reset the time variable t. This would help with the loop problem as well.
PanPalitta commented 8 years ago

I've tried to implement (2) but my meager knowledge in preprocessor directive is not enough to do so right now. I'm wondering if you have a suggestion on how to change the value of loss in the fitness function depending on where in main function it is called.

I will refocus on (3) and try to generalize it as much as possible before starting the documentation of main.cpp.

peterwittek commented 8 years ago

As for (1), the number of variables is declining, which is very good. There are still at least a dozen or more variables which are either superfluous or belong elsewhere (for example, prev_dev and new_dev). Please reduce the variables more and move all variables which should not be in main elsewhere.

peterwittek commented 8 years ago

Then for (2), are you sure you want this? Compiler directives are a last resort. Can we not have any parameter which is to be decided at compile time?

PanPalitta commented 8 years ago

For (2), I'll actually leave it to you to decide what might be a better implementation because I honestly don't know what is best. The result I am looking for is that we want to avoid having to duplicate the code for avg_fitness in Phase class. Usually we run the code at loss=0.0, but once we get the solution, we want to test it at loss > 0 to see how it performs when there is loss. So it seems to me that the same function has to be compiled twice and connected to different part of the code. I don't know how to make the compiler do this for me, or if that it even possible.

PanPalitta commented 8 years ago

As for (1), I agree with you on moving declaration of some of the variables away from main. I'm thinking of putting them into the io.cpp as they are user-chosen variables.

peterwittek commented 8 years ago

Everything that can be changed by the user should stay in main and should be allowed to be set in the config file (see read_config_file).

Okay, now I understand the problem with (2). Normally you need a template for this. Looking at avg_fitness, we might get away with something less. Can't we have a wrapper like so?

void Phase::avg_fitness(double *soln, const int K, double *fitarray) {
    // The wrapper with the old call signature
    this->avg_fitness(soln, K, fitarray, 0.0)
}

void Phase::avg_fitness(double *soln, const int K, double *fitarray, const double loss) {
    // The actual function as before, but without internally defining a constant loss
}

Perhaps I miss something obvious, but this seems viable.

PanPalitta commented 8 years ago

The wrapper is implemented and the user-chosen variables are moved to io.cpp.

peterwittek commented 8 years ago

It is a bit strange that you hard-code the loss in Phase::fitness. You could either make the class variable loss public, or add a getter/setter pair. Otherwise, main.cpp is looking great, we can close this.