payu-org / payu

A workflow management tool for numerical models on the NCI computing systems
Apache License 2.0
21 stars 27 forks source link

Update check for load user modulefiles to allow exact match #482

Closed jo-basevi closed 3 months ago

jo-basevi commented 3 months ago

Allow checks for "modules: load:" modulefiles to allow when there's an exact match in available modules. This is to fix bug #481 where module avail for openmpi/4.1.5 includes openmpi/4.1.5 and openmpi/4.1.5-debug.

I've also added tests for parsing module avail output.

Closes #481.

@aidanheerdegen last week there was an issue with multiple modules found in /g/data/vk83/modules and /g/data/vk83/modules/access-models, even though they were the same modulefile. I noticed in the modules docs there's a module paths command that lists paths of available modulefiles matching a pattern so for the above, it would still evaluate to the same path. There's a couple of differences to module avail:

If it's unlikely to have a case where module/version can be accessed from two different modulepaths and are different, e.g.

/path/to/modulefiles:
test-module/1.0.0
/another/module/path:
test-module/1.0.0

The check could be changed from modules_avail.count(modulefile) != 1 to modulefile not in modules_avail in this PR. Maybe this be moved to separate to avoid blocking this PR?

pep8speaks commented 3 months ago

Hello @jo-basevi! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2024-08-13 23:31:54 UTC
coveralls commented 3 months ago

Coverage Status

coverage: 52.612% (+0.3%) from 52.281% when pulling bfecc128ea32701151ae9abbc0d524f0a3a24b63 on ACCESS-NRI:481-fix-payu-module-uniqueness-check into 83001a2b45ef786e178894f77dd9fbc0878e73c1 on payu-org:master.

aidanheerdegen commented 3 months ago

last week there was an issue with multiple modules found in /g/data/vk83/modules and /g/data/vk83/modules/access-models, even though they were the same modulefile

That was an implementation bug. We've removed the access-models subdirectory to fix it. So it isn't a use case we need to care about or support. Does that help?

jo-basevi commented 3 months ago

Thanks for the review!

That was an implementation bug. We've removed the access-models subdirectory to fix it. So it isn't a use case we need to care about or support. Does that help?

Yes, that does help thanks. Ok so don't have to worry about using module paths or changing the implementation in that case.