gregmoille / pyLLE

Lugiato Lefever Equation Solver in Python/Julia
Other
66 stars 39 forks source link

More flexibility of Windows Julia version #37

Closed DanHickstein closed 2 years ago

DanHickstein commented 2 years ago

Hi Greg,

Thanks again for your great work on this!

It took me a little while to get the example notebook running today, because I am on windows with Julia 1.8.0 and the Julia path was hardcoded to

julia = os.path.expanduser('~') + '\\AppData\\Local\\Julia-1.1.1\\bin\\julia.exe'

but I had julia installed at:

C:\Users\DanielHickstein\AppData\Local\Programs\Julia-1.8.0\bin\julia.exe

(version number is different, and "Programs" directory is new). The error was confusing: FileNotFoundError: [WinError 2] The system cannot find the file specified, so it took me a while to even realize that was going wrong.

When I installed Julia on windows, there is an option in the installed to include Julia on the path. So, perhaps it's easier to change the readme to ask the user to add Julia to the path for all systems (already required on MacOS and Linux) and just set

julia = 'julia'

for all systems.

I also included a try/except block to check to see if python can successfully call Julia, and raise a more helpful error message if it cannot. I imagine that this will be a fairly common scenario for new users.

Also, I couldn't figure what the following code-block in the setup.py is doing. It doesn't seem to be executing when I install pyLLE, so perhaps it can be removed:

class MyInstall(install):
    def run(self):
        install.run(self)
        for ii in range(20):
            print('-'*10)
        if sys.platform == 'darwin':
            julia = 'julia'
        if sys.platform == 'linux2':
            julia = 'julia'
        if sys.platform == 'win32':
            julia = os.path.expanduser('~') + '\\AppData\\Local\\Julia-1.1.1\\bin\\julia.exe'
        sub.call([julia, 'InstallPkg.jl'])
gregmoille commented 2 years ago

Thanks Dan! Developing on Linux and Mac I kind of forgot this old ad-hoc solution that was there in the solver, thanks for fixing it and modifying the readme!

DanHickstein commented 2 years ago

Thanks Greg! And thanks for being receptive to PRs. Looking forward to using the code more.