Skip to content

Conversation

vsoch
Copy link
Member

@vsoch vsoch commented Feb 5, 2022

if singularity honors SINGULARITY_BINDMOUNT (at least since 3.0) and we are also directly specifying it,it could be provided twice to lead to the warning message. This change will test this theory for singularity by removing the explicit bind with -B

Signed-off-by: vsoch vsoch@users.noreply.github.com

if singularity honors SINGULARITY_BINDMOUNT (at least since 3.0) and we are
also directly specifying it,it could be provided twice to lead to the warning
message. This change will test this theory for singularity by removing the explicit bind
with -B

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
@vsoch vsoch mentioned this pull request Feb 5, 2022
@vsoch vsoch merged commit ff8cd90 into main Feb 7, 2022
@vsoch vsoch deleted the remove-double-bind branch February 7, 2022 11:11
@marcodelapierre
Copy link
Contributor

marcodelapierre commented Feb 14, 2022

Hi @vsoch ,
I think this change should be undone in the main codebase.

I understand you did it to test the hypothesis of the double bind mounting, however...

I think that if the sys.admin./user defines bindpaths in settings.yml, this should be respected by SHPC in the generated modulefile, not ignored; otherwise it sounds like inconsistent behaviour to me.

It is up to the person defining the settings to double-check what's already defined in the environment by SINGULARITY_BINDPATH/SINGULARITY_BINDMOUNT, and define bindpaths accordingly.

Does it make sense to you?

@vsoch
Copy link
Member Author

vsoch commented Feb 14, 2022

I think that if the sys.admin./user defines bindpaths in settings.yml, this should be respected by SHPC in the generated modulefile, not ignored; otherwise it sounds like inconsistent behaviour to me.

It should still be respected, the only difference is that it's used by setting an environment variable (instead of the implicit bind - having both let to the double bind as an error). We could reverse that and get rid of the envar and use the explicit bind if that is causing an issue.

@marcodelapierre
Copy link
Contributor

marcodelapierre commented Feb 14, 2022

Oh, sorry @vsoch ,
I hadn't realised the singularity templates also define SINGULARITY_BINDPATH themselves!!

Right now, though, I have just realised the implementation is with setenv, which means that anything preset in the environment for that variable is going to be lost. I suspect we need to do either of the following:

  1. for SINGULARITY_BINDPATH, change setenv to the prepend syntax, making sure we state the comma , delimiter
  2. instead of SINGULARITY_BINDPATH, use -B in the command line, which will add the paths to the ones already provided by the variable in the host (if defined)

Have you got any preferences between 1 and 2?
If it were me, I would probably go for 2, which is slightly more self-contained as it's part of the alias, and thus with less chances of unexpected spurious interactions with respect to shell environment changes (e.g. accidental changes to the variable by other, non-SHPC modules ..).

I am already working on SHPC today, so I can quickly shoot a PR for the one you prefer.

@vsoch
Copy link
Member Author

vsoch commented Feb 14, 2022

I think I like how the envars are easier on the eyes in the template, but you’re right it would overwrite something already set. But if you think the original implementation (with bind) is best I am good with that too. Choose the one you like best. And happy Valentine’s Day!

@marcodelapierre
Copy link
Contributor

All right, so I liked your point on the readability of using the variable, so I attempted to keep it, and use prepend_path/prepend-path , by also specifying the comma delimiter.

Unfortunately, this syntax didn't work with neither Lmod or Modules, considering I am using slightly older versions: Lmod 7.6.1 and Modules 3.2.10.6.
I guess this may be the case also in other computing centre, and it's not something that gets updated often if this is the case.

So I ended up re-adding -B and dropping the variable, see PR #494

@marcodelapierre
Copy link
Contributor

I think I like how the envars are easier on the eyes in the template, but you’re right it would overwrite something already set. But if you think the original implementation (with bind) is best I am good with that too. Choose the one you like best. And happy Valentine’s Day!

Oh, and happy Valentine's day to you, too!

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

Successfully merging this pull request may close these issues.

2 participants