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

get_exe_path is a bit noisy + question #359

Open
dbrakenhoff opened this issue Jun 27, 2024 · 9 comments
Open

get_exe_path is a bit noisy + question #359

dbrakenhoff opened this issue Jun 27, 2024 · 9 comments

Comments

@dbrakenhoff
Copy link
Collaborator

I'm getting a lot of this message (shown below) in my script, every time I call nlmod.util.get_exe_path(). When I pass a version_tag, it still logs messages. Maybe DEBUG level logging is sufficient, or maybe this message isn't very interesting?

INFO:nlmod.util.get_flopy_bin_directories:The version of the executables will not be checked, because the `version_tag` is not passed to `get_flopy_bin_directories()`.

Also a question, I just want to use the modflow binaries downloaded using flopy, how do I get nlmod to find that file? The binaries are available on my path so typing mf6 in my terminal gets me the right mf6 version. From my environment (where nlmod does not have any binaries downloaded), nlmod.util.get_exe_path() is now preferring its own binary directory in different environments instead of the flopy directory that's included in my PATH environment variable. I thought maybe this had to do with editable installs in different environments, but it also found the binaries in an environment where I installed nlmod with pip install ..

Chances are fairly large that I've just messed up my environments too much, but I wanted to check here whether anyone else has observed similar behavior?

Steps to maybe reproduce:

  1. Download flopy binaries using get-modflow :flopy
  2. Add flopy bin directory to PATH
  3. Install nlmod in two Python environments
  4. Download binaries in one Python environment
  5. Activate the other Python environment.
  6. Start python and import nlmod to run nlmod.util.get_exe_path("mf6").
  7. Which mf6 is returned?
@bdestombe
Copy link
Collaborator

bdestombe commented Jun 28, 2024

I'm getting a lot of this message (shown below) in my script, every time I call nlmod.util.get_exe_path(). When I pass a version_tag, it still logs messages. Maybe DEBUG level logging is sufficient, or maybe this message isn't very interesting?

INFO:nlmod.util.get_flopy_bin_directories:The version of the executables will not be checked, because the `version_tag` is not passed to `get_flopy_bin_directories()`.

Good point on lowering the debug level! Plus, maybe we should change a default value to get_exe_path(.., version_tag="latest").

Also a question, I just want to use the modflow binaries downloaded using flopy, how do I get nlmod to find that file? The binaries are available on my path so typing mf6 in my terminal gets me the right mf6 version. From my environment (where nlmod does not have any binaries downloaded), nlmod.util.get_exe_path() is now preferring its own binary directory in different environments instead of the flopy directory that's included in my PATH environment variable. I thought maybe this had to do with editable installs in different environments, but it also found the binaries in an environment where I installed nlmod with pip install ..

Chances are fairly large that I've just messed up my environments too much, but I wanted to check here whether anyone else has observed similar behavior?

Steps to maybe reproduce:

  1. Download flopy binaries using get-modflow :flopy
  2. Add flopy bin directory to PATH
  3. Install nlmod in two Python environments
  4. Download binaries in one Python environment
  5. Activate the other Python environment.
  6. Start python and import nlmod to run nlmod.util.get_exe_path("mf6").
  7. Which mf6 is returned?

Currently, the executable that is last installed on your system and matches the version_tag and repo(if version_tag is not None). It doesn't discriminate against different environments. Why did you perform step 4?

Thus since you explicitly downloaded the binaries again with step 4, I would presume the path of the last install in step 4. The executables installed during the first step would the the ones used if step 4 was not performed. If not, something is off. Could you have a look at this file: "flopy_metadata_fp = flopy_appdata_path / "get_modflow.json"? The last in line that matches version_tag and repo would be the path returned.

We could change this behavior of course.

@bdestombe
Copy link
Collaborator

PATH is not considered here

@bdestombe
Copy link
Collaborator

bdestombe commented Jul 2, 2024

Wait, this was not completely true. The following conditional is there as well:

    if nlmod_bindir in flopy_bindirs:
        return nlmod_bindir

    if flopy_bindirs:
        # Get most recent directory
        return flopy_bindirs[-1]

    # Else download the executables
    if download_if_not_found:

@dbrakenhoff
Copy link
Collaborator Author

Alright, I understand it better now :).

Good point on lowering the debug level! Plus, maybe we should change a default value to get_exe_path(.., version_tag="latest").

This sounds logical to me, and is more explicit.

As for your question:

Why did you perform step 4?

I was curious whether there were new binaries available in a certain environment so I quickly used nlmod functions to download the latest ones for that environment. Then in the other environment I was running some models using the flopy binaries, but using nlmod.get_exe_path("mf6") pointed to the binaries from that first nlmod environment, which I did not expect.

It doesn't discriminate against different environments.

I thought environments had be respected also for the binaries, but I understand now it just plucks the most recent location from the flopy metadata JSON file (given no binaries were installed in the current active environment)? So the philosophy is more along the lines, "you asked for a specific version, so I'll give you one that matches, regardless of where it's located".

The one worry I have is that nlmod.util.get_exe_path() without a version tag (and no binaries installed in your current nlmod environment) just gives you the most recent mf6 that was installed. But I guess being explicit in the version you want, or always downloading binaries into your current environment is good modeling practice.

And if I want to override that behavior and find a specific binary, I can get nlmod.get_exe_path() to find my flopy binaries instead of any nlmod ones by passing a bindir.

@bdestombe
Copy link
Collaborator

The one worry I have is that nlmod.util.get_exe_path() without a version tag (and no binaries installed in your current nlmod environment) just gives you the most recent mf6 that was installed. But I guess being explicit in the version you want, or always downloading binaries into your current environment is good modeling practice.

What is you view on:

  • For best reproducibility we should indeed pin the mf versions. Should we alter the examples such that we explicitly select mf versions? An argument against is that those mf versions seem to just get more stable and remain backward compatible, if it is not a problem now, there is not need to solve it.
  • Should we introduce a default version_tag in ./nlmod/version.py and we update it in nlmod with every version bump of nlmod as we go? This would help creating more reproducible results without bothering the user. All they need to do is pin the version of nlmod, which was already needed.
  • Alternatively, we push the user to using the latest version get_exe_path(.., version_tag="latest"), where ever it may be installed on the system. We assume newer is better and this way the user is guaranteed to use the latest.

An additional question to all:

  • Instead of generating a path for the executable, I would like only the attributes version_tag and repo to be stored in the model_ds. The path to the executable+downloading happens when running the simulation. The keeps the to_modelds() command a bit leaner, only stores the information that affects the outcome. This would make model_ds a bit more transferrable.

@dbrakenhoff
Copy link
Collaborator Author

I'm for defaulting to "latest" and documenting that it is a good idea to think about pinning mf6 versions for projects. I think pinning versions in nlmod will be a bit too annoying in development. It would be nice to be able to easily figure out what version_tag "latest" corresponds to, and/or which version of mf6 is contained within a specific version tag.

Instead of generating a path for the executable, I would like only the attributes version_tag and repo to be stored in the model_ds. The path to the executable+downloading happens when running the simulation. The keeps the to_modelds() command a bit leaner, only stores the information that affects the outcome. This would make model_ds a bit more transferrable.

I do like seeing the path in my model dataset, that gives me confidence that it's using the right binary to run the model. Maybe that's old fashioned thinking though? I'm fine with adding repo/version tag to make it more transferable though, as long as I don't have to type more 😇 .

@bdestombe
Copy link
Collaborator

So that makes it 1 vs. 1 ;)

Team full path Team version_tag+repo
1 1

@OnnoEbbens, @rubencalje would you like to cast a vote?

I don't think we should do both as they can get out of sync.

@dbrakenhoff
Copy link
Collaborator Author

I'd be happy to switch my vote if nlmod.util.get_exe_path("mf6", ds=ds) gives me the path to an executable 😇

Also it would be nice to quickly extract the mf6 version from some version_tag: nlmod.util.get_mf6_version(version_tag="latest")?

@dbrakenhoff
Copy link
Collaborator Author

BTW, I lowered the log level to DEBUG in 45c25be. Leaving this issue open for the other discussion on ds.attrs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants