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.

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

Check parallel option sent to fmincon #83

Closed
H0R5E opened this issue Mar 28, 2020 · 4 comments · Fixed by #101
Closed

Check parallel option sent to fmincon #83

H0R5E opened this issue Mar 28, 2020 · 4 comments · Fixed by #101
Assignees
Labels
bug Something isn't working
Milestone

Comments

@H0R5E
Copy link
Member

H0R5E commented Mar 28, 2020

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.

@H0R5E H0R5E added cleanup/useability Improvements to structure, etc. high priority labels Mar 28, 2020
@H0R5E H0R5E mentioned this issue Mar 28, 2020
7 tasks
@ryancoe ryancoe added this to the v0 release milestone Mar 30, 2020
@ryancoe ryancoe self-assigned this Mar 30, 2020
@ryancoe
Copy link
Member

ryancoe commented Mar 31, 2020

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.

@ryancoe ryancoe closed this as completed Mar 31, 2020
@H0R5E
Copy link
Member Author

H0R5E commented Mar 31, 2020

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 H0R5E reopened this Mar 31, 2020
@H0R5E H0R5E added bug Something isn't working and removed cleanup/useability Improvements to structure, etc. labels Mar 31, 2020
@H0R5E
Copy link
Member Author

H0R5E commented Mar 31, 2020

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

@H0R5E H0R5E assigned H0R5E and unassigned ryancoe Mar 31, 2020
@H0R5E
Copy link
Member Author

H0R5E commented Mar 31, 2020

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.

@H0R5E H0R5E linked a pull request Mar 31, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants