cmd-ntrf / jupyter-lmod

Jupyter plugin that provides a tab for TACC Lmod (https://github.com/TACC/Lmod)
MIT License
28 stars 13 forks source link

Load/Unload issue depending on naming of module file #47

Closed dr-br closed 2 years ago

dr-br commented 2 years ago

Depending on the naming of the Lmod module file, the module is listed under "LOADED MODULES" or not. When loading the module via the "Load" button, it gets loaded. However, as it does not appear in the "LOADED MODULES" list, it can't be unloaded.

Example: With the module file 9-test.lua everything works as expected, with 9.lua it does not. Both files are a symlink to exactly the same module file.

As in #26 we already identified a regex as the root of all evil, I would eventually point to lines 15+16 in this file: https://github.com/cmd-ntrf/jupyter-lmod/blob/main/lmod/__init__.py

Thanks in advance :)

dr-br commented 2 years ago

no-hidden: ^(.*?\/[^.][^\/]*)$ all: ^(.*?\/[^\/]*)$

While these are the more convenient regexes for MODULE_REGEX and MODULE_REGEX_NO_HIDDEN, there is still the requirement for the version to be at least 2 \w Symbols...

cmd-ntrf commented 2 years ago

Can you provide an example with the complete path for the module? Or do you have module with just a number as the name at the root of the module path?

dr-br commented 2 years ago

Module path is e.g. category/software/version version={12, 112-test, t- } is working version = {1, t} is not working

No module with just a number as the name at root.

So, e.g. jupyter/minimal/2022-01-11 is fine, while jupyter/minimal/2 is not. Version has to consist of at least 2 symbols.

It seems not to be the regex (I even checked *) for MODULE_REGEX.

cmd-ntrf commented 2 years ago

~I am currently unable to reproduce.~

I have this barebone module structure:

modules $ tree
.
├── jupyter
│   └── minimal
│       └── 2.lua
└── rstudio-server
    └── 1.4.43.lua
image

~jupyter/minimal/2 shows up as expected.~

I apologize, I misread your bug. I can reproduce the issue with the loading button.

cmd-ntrf commented 2 years ago

The issue is with MODULE_REGEX_NO_HIDDEN which cannot match jupyter/minimal/2, leading jupyter-lmod to think wrongfully that it is an hidden module.

cmd-ntrf commented 2 years ago

@dr-br : can you try branch hidden_regex and confirm it solves your issue.

dr-br commented 2 years ago

Yes, but now there is another issue:

grafik

As you can see, my openfoam module loads among others mpi/openmpi/4.0 (right side, module list), but is only displayed as mpi/openmpi on the left hand side. I can unload this module with the button, (module list in terminal is empty) but it remains visible within the "LOADED MODULES" listing :(

cmd-ntrf commented 2 years ago

After loading openfoam with the command-line, could you echo the content of $LOADEDMODULES and paste it here:

echo $LOADEDMODULES
cmd-ntrf commented 2 years ago

I have made a few improvements to the PR. I am not sure why the version of the mpi/openmpi/4.0 module would be cut and I have not been able to reproduce locally.

dr-br commented 2 years ago
[sam@supercomputer sam]$ module li

Currently Loaded Modules:
  1) jupyter/minimal/2022-01-11

[sam@supercomputer sam]$ echo $LOADEDMODULES
jupyter/minimal/2022-01-11:mpi/openmpi

Obviously, the LOADEDMODULES is strange in our settings (version is not displayed for mpi). I will check with the responsible guys.

dr-br commented 2 years ago

Update: We located the issue with diverging module li and $LOADEDMODULES. So from this point of view, this ticket could be closed, the regex-fix and your other modifications solve this issue. However, having a look at 1, it seems that the variable $LOADEDMODULES is only for compatibility reasons with Tmod. It would be safer to eventually use module -t list instead of $LOADEDMODULES. In our specific case, we could not rely on this variable.

So instead of loadedmodules = os.environ.get('LOADEDMODULES', '') grab the output of module -t list.

EDIT:
I just realized, that you introduced loadedmodules = os.environ.get('LOADEDMODULES', '') in this PR. I would be in favor of reverting this ;)

cmd-ntrf commented 2 years ago

I agree. I reverted to use module -t list as a source for loaded module.

cmd-ntrf commented 2 years ago

The fix has been released in 2.0.4.

Thank you for reporting this!

dr-br commented 2 years ago

Thank YOU! Works like a charm.

dr-br commented 2 years ago

I think the version string in setup.py still needs to be updated.

cmd-ntrf commented 2 years ago

Released again as 2.0.5