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

bug: location of mf6 executable using relative path #1633

Closed
mbakker7 opened this issue Nov 18, 2022 · 25 comments · Fixed by #1957
Closed

bug: location of mf6 executable using relative path #1633

mbakker7 opened this issue Nov 18, 2022 · 25 comments · Fixed by #1957
Labels
Milestone

Comments

@mbakker7
Copy link
Contributor

mbakker7 commented Nov 18, 2022

I am stumbling with flopy to locate the mf6 executable on my Mac.

I define the location with a relative path, as per documentation (Relative path to MODFLOW 6 executable from the simulation working folder).

exe_name='../bin/mf6'

But when I run the model I get:

FileNotFoundError: [Errno 2] No such file or directory: '../bin/mf6'

When in the next code cell I do:

ls ../bin/

I get

mf6* mp7*

So mf6 clearly exist in the right location and is executable. I can even run it

!../bin/mf6

Gives

ERROR REPORT: 1. mf6: mfsim.nam is not present in working directory.

When I move this mf6 to the same directory as the notebook and change exe_name to

path = os.getcwd()
exe_name=path + '/mf6'

But then it is an absolute path.

My colleague claims that

exe_name='../bin/mf6.exe'

works on his Windows machine.

Is this a Mac problem?

@mbakker7 mbakker7 added the bug label Nov 18, 2022
@mbakker7 mbakker7 changed the title bug: bug: location of mf6 executable using relative path Nov 18, 2022
@mbakker7
Copy link
Contributor Author

Suggested solution:

In flopy.mf6.MFSimulation, replace self.exe_name = exe_name by os.path.abspath(exe_name).
(and import os)

@spaulins-usgs
Copy link
Contributor

@mbakker7, I like your solution, though it should be implemented in mbase.py run_model, and only if the attempt to find the executable using shutil.which fails.

@spaulins-usgs
Copy link
Contributor

@mbakker7, I added code that tries converting the executable path to an absolute path (os.path.abspath) when all else fails. Also, updated the comments for the documentation. The changes are checked in to the develop branch. Please let me know if this fixes the problem.

@mbakker7
Copy link
Contributor Author

Sorry, @spaulins-usgs, still doesn't work. My relative path is ../bin/mf6. When I specify that, I get the error message FileNotFoundError: [Errno 2] No such file or directory: '../bin/mf6'. The simulation runs fine when I specify exe_name=os.path.abspath('../bin/mf6').

Is there a way to ask flopy what executable (full path) it will use, so I can check where it goes wrong?

@spaulins-usgs
Copy link
Contributor

@mbakker7, to output the executable path being used try setting the silent flag to False:

sim.run_simulation(silent=False)

I am not 100% sure what is wrong and I do not have a Mac to test this, though I do have some ideas. If you are willing to test these, there are some potential fixes on my fork of flopy here:

flopy

The only modified file in this fork is flopy/mbase.py

Please let me know if these updates fix the issue.

@mbakker7
Copy link
Contributor Author

@spaulins-usgs, sorry for the late reply. I was on break for a while, but I am back and determined to help fix this issue.

The message I get when running modflow is: FloPy is using the following executable to run the model: ../bin/mf6. Now that is indeed the path I provide. When I run ls ../bin/mf6, I nicely get back ../bin/mf6*, so the file exist. And the previously discussed use of os.path.abspath works fine, so the error must be somewhere in mfbase, where the executable path is stored. Surely other developers (@jdhughes , @langevin-usgs) must run into similar problems as I thought they use Macs.

For your information, the full trace back is:

---------------------------------------------------------------------------
FileNotFoundError                         Traceback (most recent call last)
Input In [5], in <cell line: 90>()
     88 # Write input files and solve
     89 sim.write_simulation(silent=True)
---> 90 success, _ = sim.run_simulation(silent=False) 
     91 if success == 1:
     92     print('Model solved successfully')

File ~/git/flopy/flopy/mf6/modflow/mfsimulation.py:1467, in MFSimulation.run_simulation(self, silent, pause, report, normal_msg, use_async, cargs)
   1465     else:
   1466         silent = True
-> 1467 return run_model(
   1468     self.exe_name,
   1469     None,
   1470     self.simulation_data.mfpath.get_sim_path(),
   1471     silent=silent,
   1472     pause=pause,
   1473     report=report,
   1474     normal_msg=normal_msg,
   1475     use_async=use_async,
   1476     cargs=cargs,
   1477 )

File ~/git/flopy/flopy/mbase.py:1743, in run_model(exe_name, namefile, model_ws, silent, pause, report, normal_msg, use_async, cargs)
   1740         argv.append(t)
   1742 # run the model with Popen
-> 1743 proc = Popen(argv, stdout=PIPE, stderr=STDOUT, cwd=model_ws)
   1745 if not use_async:
   1746     while True:

File ~/opt/anaconda3/lib/python3.9/subprocess.py:951, in Popen.__init__(self, args, bufsize, executable, stdin, stdout, stderr, preexec_fn, close_fds, shell, cwd, env, universal_newlines, startupinfo, creationflags, restore_signals, start_new_session, pass_fds, user, group, extra_groups, encoding, errors, text, umask)
    947         if self.text_mode:
    948             self.stderr = io.TextIOWrapper(self.stderr,
    949                     encoding=encoding, errors=errors)
--> 951     self._execute_child(args, executable, preexec_fn, close_fds,
    952                         pass_fds, cwd, env,
    953                         startupinfo, creationflags, shell,
    954                         p2cread, p2cwrite,
    955                         c2pread, c2pwrite,
    956                         errread, errwrite,
    957                         restore_signals,
    958                         gid, gids, uid, umask,
    959                         start_new_session)
    960 except:
    961     # Cleanup if the child failed starting.
    962     for f in filter(None, (self.stdin, self.stdout, self.stderr)):

File ~/opt/anaconda3/lib/python3.9/subprocess.py:1821, in Popen._execute_child(self, args, executable, preexec_fn, close_fds, pass_fds, cwd, env, startupinfo, creationflags, shell, p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite, restore_signals, gid, gids, uid, umask, start_new_session)
   1819     if errno_num != 0:
   1820         err_msg = os.strerror(errno_num)
-> 1821     raise child_exception_type(errno_num, err_msg, err_filename)
   1822 raise child_exception_type(err_msg)

FileNotFoundError: [Errno 2] No such file or directory: '../bin/mf6'

@mbakker7
Copy link
Contributor Author

I think I found the problem.

In mfbase, you try the following:

    exe = which(exe_name)
    if exe is None:
        if exe_name.lower().endswith(".exe"):
            # try removing .exe suffix
            exe = which(exe_name[:-4])
    if exe is None:
        # try abspath
        exe = which(os.path.abspath(exe_name))

When a relative path is passed to the which function, it happily returns the same relative path. Which is not what you want (on a Mac), as it needs to be an absolute path.

from shutil import which
which('../bin/mf6')

returns

'../bin/mf6'

Hence, exe is never None, and the absolute path is never set. Now this may work different on Windows. Or simply running an executable with a relative path may work on Windows, I don't know. But on a Mac running with a relative path doesn't work. Hope this helps.

@jlarsen-usgs
Copy link
Contributor

jlarsen-usgs commented Jan 12, 2023

@mbakker7

I just reproduced this same error on windows. When I supply a relative path without the extension (some_rel_path\mp7) flopy crashes ( with modpath7), however if I supply the extension (some_rel_path\mp7.exe on windows) it runs. Can you test this quirk on mac? It seems like we need to identify the platform and provide the extension prior to calling which()

@mbakker7
Copy link
Contributor Author

I can confirm the same error on Windows: If you specify a relative path you must also specify '.exe'. This is odd, as flopy just removed the requirement to add .exe. But with a relative path, it is still required.

An executable program on a Mac doesn't have an extension but a special mode flag that the OS recognizes.

May I (re)suggest to always use os.path.abspath in flopy on the provided path? What is the downside to always use os.path.abspath?

@spaulins-usgs
Copy link
Contributor

@mbakker7, I think it would be fine to use os.path.abspath as long as the exe_name supplied is more than just an executable name (i.e. it is either an absolute path or contains at least one folder name in it). However, if just an executable name is supplied, FloPy should resolve the full path to the executable by looking for the executable in the Windows/Mac defined paths (Windows/Mac "PATH" variable). The which command will not do this with an absolute path. For example, on Windows which(os.path.abspath("MF6.exe")) will not search the "PATH" variable paths for "MF6.exe", but which("MF6.exe") will. Therefore, I think the behavior should be:

  if <exe_name is just a file name>:
        exe = which(exe_name)
  else:
        exe = which(os.path.abspath(exe_name))

@spaulins-usgs
Copy link
Contributor

@mbakker7, I implemented the behavior described in my post above here:

https://github.com/scottrp/flopy/blob/develop/flopy/mbase.py

Let me know if that works for you.

@mbakker7
Copy link
Contributor Author

Still won't work, because os.path.dirname('../bin/mf6') gives '../bin' and not an empty string.

I understand you want to use which to search the path. What I suggest is to remove

   if os.path.dirname(exe_name) == "":
         exe = which(exe_name)
         if exe is None:
             if exe_name.lower().endswith(".exe"):
                 # try removing .exe suffix
                 exe = which(exe_name[:-4])
                 if exe is not None:
                     exe_name = exe_name[:-4]
     else:
         exe = os.path.abspath(exe_name)

and replace it with the simple

os.path.abspath(shutil.which(exe_name))

Not sure what to do with the exe extension. You can remove that on Windows if you want (I cannot test that). Will this work for you? It works for me!

@mwtoews
Copy link
Contributor

mwtoews commented Jan 15, 2023

I too can confirm that this does not work on Windows:

sim = flopy.mf6.MFSimulation.load("mfsim.nam", exe_name=r"..\..\dev\wrdapp\bin\mf6")
sim.run_simulation()
# ....
# Exception: The program ..\..\dev\wrdapp\bin\mf6 does not exist or is not executable.

but adding .exe allows it to work. @spaulins-usgs does your branch have this fix? Otherwise I can create one.

I removed .exe from most of the package, since it isn't needed outside flopy, e.g.:

from subprocess import Popen, PIPE
r = Popen(r"..\..\dev\wrdapp\bin\mf6", stdout=PIPE, stderr=PIPE)
stdout, stderr = r.communicate()
assert "Normal termination of simulation" in stdout.decode()

Windows doesn't need file extensions for executables, and should accept absolute or relative paths.

@mbakker7
Copy link
Contributor Author

I think I am getting to the root of the problem.

from subprocess import Popen, PIPE
r = Popen("../bin/mf6", stdout=PIPE, stderr=PIPE)
print(r.communicate())

runs mf6 fine on a mac. But if I specify a cwd in Popen (where flopy specifies, I think, the specified workspace modelws, not sure what that is called in the flopy codebase), I get an error No such file or directory: '../bin/mf6'

from subprocess import Popen, PIPE
r = Popen("../bin/mf6", stdout=PIPE, stderr=PIPE, cwd='model1') # gives error
print(r.communicate())

Popen simply uses the specified current working directory and the relative path wrt the workspace should be specified, so the following code works (not that I think we should do it this way).

from subprocess import Popen, PIPE
r = Popen("../../bin/mf6", stdout=PIPE, stderr=PIPE, cwd='model1') # note modified relative path
print(r.communicate())

So I tried to specifiy "../../bin/mf6" in flopy, but it still can not find mf6, while it really should. Not sure then what flopy really specifies when doing Popen.

@spaulins-usgs
Copy link
Contributor

spaulins-usgs commented Jan 18, 2023

@mbakker7, sorry it looks like I misread how the code is suppose to work. I did not originally write this section code, so I am trying to figure out its behavior just like you. I thought when the code was doing this:

exe = which(exe_name)

That it was then using the resulting "exe" variable when calling Popen, but it is actually using exe_name in the Popen call. My change therefore did not actually do anything. I still think this is the correct general approach, I was just setting the wrong variable. I fixed this and tested it with various exe_name values on Windows (just the exe name with extension, just the exe name without extension, relative path with extension, relative path without extension, full path with extension, full path without extension), and it works for all of them. The updated code is here:

https://github.com/scottrp/flopy_exe_path/blob/develop/flopy/mbase.py

Regarding replacing the code with:

os.path.abspath(shutil.which(exe_name))

This fails when a partial path to the executable without the .exe extension is supplied ("bin\mf6"). In this case shutil.which returns None and abspath raises an exception because of this.

Anyway, I think much of the confusion here (at least the confusion on my part) is that the which call is not actually doing anything other than testing for the existence of the executable. The fix I am now proposing both fixes this existence test and converts any relative paths to absolute for Popen.

@mwtoews, the problem is that "which" will not detect a partial path to the executable unless the extension is included. The updated code fixes this problem.

@mbakker7
Copy link
Contributor Author

The updated code works on my Mac. For both an absolute path but now also for a relative path!
Ready to submit a PR as far as I am concerned.
Thanks for all your help!

langevin-usgs pushed a commit that referenced this issue Feb 15, 2023
* fix(exe path): FloPy now correctly resolves relative paths to mf6 executable (#1633)

* feat(solvers): support for multiple solver types #1706

* Revert "fix(exe path): FloPy now correctly resolves relative paths to mf6 executable (#1633)"

This reverts commit 14335cf.

* Update mfsimulation.py

feat(solver support): rolling back ims deprecation warning

---------

Co-authored-by: scottrp <45947939+scottrp@users.noreply.github.com>
spaulins-usgs added a commit that referenced this issue Mar 2, 2023
…cutable (#1633) (#1727)

* feat(exe path): support for relative paths

---------

Co-authored-by: scottrp <45947939+scottrp@users.noreply.github.com>
wpbonelli added a commit to wpbonelli/flopy that referenced this issue Mar 3, 2023
wpbonelli added a commit that referenced this issue Mar 3, 2023
- support pathlike or str paths in public APIs
- add tests for supported path arguments
- update docstrings and add type hints
- update some notebooks to use pathlib
- update most autotests to use pathlib
- add verbose flag to more util functions
- add sim_path property to MFSimulation
- expand run_model() tests to cover #1633
- update resolve_exe(), add tests
@wpbonelli
Copy link
Contributor

The updated exe handling code is in develop with #1727, and #1730 added tests for relative paths, abs paths, and named exes. I think the failure case is covered by a test but wanted to double check that this is resolved on your end @mbakker7 ?

@wpbonelli
Copy link
Contributor

tentatively closing as this should now be fixed and covered by tests

@mbakker7
Copy link
Contributor Author

Just wanted to confirm that the current develop version works fine with a relative path on my Mac. Thanks again for fixing this!

The updated exe handling code is in develop with #1727, and #1730 added tests for relative paths, abs paths, and named exes. I think the failure case is covered by a test but wanted to double check that this is resolved on your end @mbakker7 ?

@mbakker7 mbakker7 reopened this Sep 18, 2023
@mbakker7
Copy link
Contributor Author

It appears that in the current version of flopy, the executable still has to be specified as mf6.exe. So with the .exe extension. Otherwise a flopy script doesn't run on windows. Funny enough, even with the .exe extension, the script runs on a Mac, so flopy scrapes off .exe somehow.
Shouldn't it be that .exe is never required, and flopy adds .exe to run the script on windows?

@wpbonelli
Copy link
Contributor

wpbonelli commented Sep 19, 2023

@mbakker7 I'm unable to reproduce this on Windows with 3.4.2 or the develop branch. Provided an mf6 executable is on the system path, passing exe_name="mf6" to both MFSimulation.run_simulation() and mbase.run_model() works on my end.

Funny enough, even with the .exe extension, the script runs on a Mac, so flopy scrapes off .exe somehow.

That is done here, the motivation being to accommodate flopy scripts which were developed on windows and then moved to a different platform.

Shouldn't it be that .exe is never required, and flopy adds .exe to run the script on windows?

Yes, the extension should never be required. Flopy does not currently add .exe on windows internally either — my understanding is it is not needed unless PATHEXT has a nonstandard value (by default, .exe should be included since at least WinXP)

Would it be possible to share the script you are seeing the issue with, as well as the Windows and Python versions, the value of PATHEXT, and whether shutil.which("mf6") succeeds in the same environment?

@mbakker7
Copy link
Contributor Author

Thanks for looking into this (again), @wpbonelli
I like it a lot that mf6.exe now also works on my Mac! I encountered that by accident (forgot to change it when somebody sent me a notebook from Windows). Regarding the issue on Windows, we specify the relative path to the mf6 executable and I have been told by @vincentpost that on Windows that still requires the .exe (we are teaching an mf6/flopy workshop together). So our simulation looks something like this:

sim = fp.mf6.MFSimulation(sim_name='model1', 
                          version='mf6', 
                          exe_name='../bin/mf6.exe', # relative path to MODFLOW executable
                          sim_ws='./model1',
                         )

@vincentpost can you confirm this and give the flopy version that you are using?

@wpbonelli
Copy link
Contributor

Hi @mbakker7 thanks for the extra info! I understand the issue now.

The .exe extension is indeed necessary if the binary is located by absolute or relative path on Windows. This is consistent with the behavior of shutil.which, but inconsistent with command prompt and powershell, where the extension is not needed.

I don't see any downsides to supporting extensionless paths — this could easily be added for the next minor version release, unless there are considerations I'm missing.

@mbakker7
Copy link
Contributor Author

Ok. So it works as designed. If I understand it correctly:

  1. On a Mac, you can specify mf6 or mf6.exe and both will work (the .exe gets stripped off when detected).
  2. On Windows, you can sometimes specify mf6, but if you use an absolute or relative path, you must add .exe. That is unfortunate, because that means that if I send a notebook from my Mac to a Windows user, (s)he has to add the .exe.

Suggested solution: Isn't it the case that mf6.exe always works on Windows? Couldn't flopy then just add .exe when it is not provided, in a similar fashion that .exe is stripped when supplied on a Mac?

@wpbonelli
Copy link
Contributor

Yes, currently the .exe is not needed if the binary is on the PATH and identified by name, but it is needed if the binary is specified by relative or absolute path. Sorry for my misleading comment above, about it never being required — I was only considering name-identification!

Suggested solution: Isn't it the case that mf6.exe always works on Windows? Couldn't flopy then just add .exe when it is not provided, in a similar fashion that .exe is stripped when supplied on a Mac?

I've opened #1957 to support this.

@wpbonelli wpbonelli added this to the 3.4.3 milestone Sep 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants