SNL-WaterPower / WecOptTool-MATLAB

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

Dependency Checker: Toolbox License #34

Closed ssolson closed 4 years ago

ssolson commented 4 years ago

Mat in Dependency Checker I believe it should be: license('test', 'Optimization Toolbox') and license('test', 'Parallel Computing Toolbox)

I do not believe we need to iterate over the names in the license.

Further, can you confirm that the optimization toolbox is truly a requirement?

H0R5E commented 4 years ago

Yeah, I think the optimization toolbox is truly a requirement...

'optimoptions' requires Optimization Toolbox.

Error in example (line 40)
options = optimoptions('fmincon',                   ...
H0R5E commented 4 years ago

Can you post me your outputs from the ver and license('test', "Optimization Toolbox") commands, please?

ssolson commented 4 years ago

ver is long but the head is below:

ver

MATLAB Version: 9.7.0.1190202 (R2019b) MATLAB License Number: 10848 Operating System: Linux 3.10.0-1062.1.2.el7.x86_64 SNL-WaterPower/WecOptTool#11 SMP Mon Sep 16 14:19:51 EDT 2019 x86_64 Java Version: Java 1.8.0_202-b08 with Oracle Corporation Java HotSpot(TM) 64-Bit Server VM mixed mode

MATLAB Version 9.7 (R2019b) Simulink Version 10.0 (R2019b) Aerospace Blockset Version 4.2 (R2019b) Aerospace Toolbox Version 3.2 (R2019b) Bioinformatics Toolbox Version 4.13 (R2019b) Communications Toolbox Version 7.2 (R2019b) Computer Vision Toolbox Version 9.1 (R2019b) ...

H0R5E commented 4 years ago

Is "Optimization Toolbox" in the ver output?

ssolson commented 4 years ago

Yes "Optimization Toolbox" is included in the list.

However, it seems that the call to license is suffcient to me. The output requested for that part is below:

>> license('test', 'Optimization Toolbox') ans = 0 >> license('test', 'Parallel Computing Toolbox') ans = 0

ssolson commented 4 years ago

Yeah, I think the optimization toolbox is truly a requirement...

'optimoptions' requires Optimization Toolbox.

Error in example (line 40)
options = optimoptions('fmincon',                   ...

I see now that fmincon is part of the toolbox as well. You are certainly right about the need for the toolbox!

H0R5E commented 4 years ago

OK, so the ver command is used to check if the toolbox is installed and the license command is used to check if it's licensed. The script should then respond accordingly. It seems that license('test', 'Optimization Toolbox') isn't working for either of us, however, as 0 implies there is no license. For me, I thought this was because I was using a demo version, but if it's not working for you either then I will try another way.

ssolson commented 4 years ago

Hey Mat,

I think its just a syntax thing. Changing the space to underscore worked for the optimization toolbox.

>> license('test', 'Optimization_Toolbox')

ans =

     1
ssolson commented 4 years ago

lol it has a magic name. Turns out this is the command we need for the parallel toolbox.

license('test','Distrib_Computing_Toolbox') 

I will go ahead and implement this. Are you okay with removing the 'DEMO' checks in that if statement?

H0R5E commented 4 years ago

Works for me too, thanks. Yes, the 'DEMO' part is irrelevant now. Note you'll need to update the README too, as it has example output with DEMO in it. Are you going to do a PR?

Also, you're up early!

ssolson commented 4 years ago

This is my schedule, you won't catch me here past 16:00!...when I can help it.

Yeah I will make the changes as do a pull request.

ssolson commented 4 years ago

I believe this is closed. After you review the merge let me know if you see any more issues to discuss here.

H0R5E commented 4 years ago

Well if it's merged then it's done, right?

ssolson commented 4 years ago

Well IMPO :) . I thought you were gone until Sunday and you could call me impatient.

H0R5E commented 4 years ago

I'm effectively gone, just being boring. We should probably ban merging our own pull requests. Did you squash and merge?

ssolson commented 4 years ago

You're probably right but I would say discourage not ban :). I did not know that squashing was the norm. Will do in the future.

H0R5E commented 4 years ago

Squash is nice because you can report the serious changes and ignore the silly typo commits.

H0R5E commented 4 years ago

Closed by SNL-WaterPower/WecOptTool_old#47