SNL-WaterPower / WecOptTool-MATLAB

WEC Design Optimization Toolbox
GNU General Public License v3.0
12 stars 9 forks source link

Check parallel option sent to fmincon #83

Closed H0R5E closed 4 years ago

H0R5E commented 4 years ago

One minor thing I noticed that may be a good idea to address, the example.m file is dependent on has "use-parallel" option set to True, but this requires the Parallel Computing Toolbox if I am not mistaken. I am unsure of whether MATLAB will default to the non-parallel case if the user does not have the toolbox. We specify in the README and html documentation that the Parallel Optimization Toolbox is optional. I looked into it a little bit, and it seems that license('test', 'Distrib_Computing_Toolbox') can test if someone has access to the toolbox, so perhaps we should place the options inside of an if statement depending on this command?

Originally posted by @zmorrell-sand in https://github.com/SNL-WaterPower/WecOptTool/issues/65#issuecomment-605332999

We need to check what happens for a user without the parallel toolbox running the example as it stands with 'UseParallel' set to true. If it errors we should consider mitigating measures as suggested.

ryancoe commented 4 years ago

I realized that my Optimization Toolbox and Parallel Computing Toolbox (PCT) are on the same network license server, so I can't test this in the way that I planned. I'm 99.99% sure that when you don't have the PCT, MATLAB will just run in serial. I know this to be the case for parfor.

https://www.mathworks.com/matlabcentral/answers/499147-how-do-i-get-parfor-to-downgrade-gracefully-to-for-when-no-parallel-seat-is-open

Closing for now and will reopen it if someone has an issue.

H0R5E commented 4 years ago

I can confirm that it glitches out in this case:

'getCurrentWorker' requires Parallel Computing Toolbox.

Error in WecOptLib.models.RM3.DeviceModel/getHydrodynamics>RM3_parametric (line
111)
worker = getCurrentWorker;

Error in WecOptLib.models.RM3.DeviceModel/getHydrodynamics (line 87)
        [hydro,rundir] = RM3_parametric(r1,r2,d1,d2);

Error in WecOptLib.models.DeviceModelTemplate/getPower (line 74)
        [hydro,rundir] = obj.getHydrodynamics(geomMode, geomParams);

Error in WecOptTool.run/obj (line 38)
        pow = -1 * RM3Device.getPower(study.spectra,          ...

Error in fmincon (line 562)
      initVals.f = feval(funfcn{3},X,varargin{:});

Error in WecOptTool.run (line 65)
    [sol,fval,exitflag,output] = fmincon(@obj,                  ...

Error in example (line 65)
WecOptTool.run(study, options);

I think the easiest way to deal with this is to take it out of the default example. I'll open a PR to this effect.

H0R5E commented 4 years ago

Strike that. I think this a bug of my own making. Investigating.

H0R5E commented 4 years ago

OK, I made a PR for this in #101. We need to be careful when we include Parallel Toolbox methods (like getCurrentWorker in this case) as they will error if it's not installed. I added a utility function (WecOptLib.utils.hasParallelToolbox) so we can check for it in the code.

If we do CI (as in #97), it might be nice to have two VMs, one with the parallel toolbox installed and one without.