Portability and reproducibility fixes

[sebastian-luna-valero]

Hi @jonesmg

Could you please have a look at these changes and let me know your thoughts?

I am running tests with this code on a new platform and I did a couple of minor changes to improve portability and reproducibility.

We have informally discussed several times about pinning the miniconda version. Here I include a change to do that.

Best regards, Sebastian

[jonesmg]

Hi @sebastian-luna-valero

I just tried this, but I ran into some trouble. I actually don't think the issue is necessarily related to these specific changes, but because I'm running it on a different system to what I've tested it on previously. I did manage to get the environment correctly built and activated, but it wasn't straightforward and the run.sh script failed.

As best I could tell it was failing because of line 75 in run.sh. I think the script was finding the existing version of conda (rather than the new miniconda installation) and thus never sourcing the conda-install/etc/profile.d/conda.sh file. I tried modifying run.sh to force it to source that file and then it seemed to work. Does that seem like a suitable fix to you? I suppose it may fail in a case where user already has exactly the right miniconda version already installed, but not in the expected directory.

I also think that the run.sh script should probably include a step to deactivate any existing conda installation at the beginning. This caused me additional problems as the first time I tried to run it my terminal still had the base environment of the existing conda installation active.

Cheers,

Mike

[sebastian-luna-valero]

Thanks for your comments, Mike.

I have now refactor the code, could you please try it again and let me know how it goes?

Here is what I think would be the ideal behavior:

• if your terminal doesn't have conda, install everything and run the pipeline
• if you already have the conda command available: do not install conda again (this is what I am trying to fix with my commit) check whether you already have a conda environment called hcg-16 and use that (assuming it is a correct one) ** if not, just create a new hcg-16 environment on your existing conda installation to run the code.

Answering your questions:

As best I could tell it was failing because of line 75 in run.sh. I think the script was finding the existing version of conda (rather than the new miniconda installation) and thus never sourcing the conda-install/etc/profile.d/conda.sh file. I tried modifying run.sh to force it to source that file and then it seemed to work. Does that seem like a suitable fix to you? I suppose it may fail in a case where user already has exactly the right miniconda version already installed, but not in the expected directory.

I think the behavior I explained above is a better approach, and I tried to reflect that in my latest commit.

I also think that the run.sh script should probably include a step to deactivate any existing conda installation at the beginning. This caused me additional problems as the first time I tried to run it my terminal still had the base environment of the existing conda installation active.

I think having conda already installed shouldn't be a problem, we just need to make sure that the hcg-16 environment exists and reuse it, or install a new one.

This is a interesting exercise!

Best regards, Sebastian

[jonesmg]

Hi Sebastian

I again got errors which I think were from the correct conda installation not being active (see below). I just pushed some changes to run.sh that made it work for me. Let me know whether you think they are suitable.

Cheers,

Mike

CommandNotFoundError: Your shell has not been properly configured to use 'conda activate'.
To initialize your shell, run

    $ conda init <SHELL_NAME>

[sebastian-luna-valero]

Thanks Mike,

And sorry I didn't test my code before pushing.

Your changes work for me in a clean environment. As I mentioned before I was concerned about doing source conda-install/etc/profile.d/conda.sh in an environment where you already had conda available. I wanted to be careful with:

if [[ "${CONDA_EXE}" ]] ; then
   # this should detect whether conda is already available, but it doesn't seem to work according to your comments
    log " Conda activated. "
else
    log " Activate conda... "
    source conda-install/etc/profile.d/conda.sh
fi

Given that your proposed changes work for you as well in an environment with conda previously enabled, I would just go ahead with that.

On the other hand, I wanted to encourage the end user to work on a dedicated folder and I added a couple of lines to the readme, could you please have a look and feel free to edit with your comments?

Best regards, Sebastian

[jonesmg]

Sounds good. I added a couple of other changes to the readme, so that the same version of miniconda is suggested for running the plot scripts only. This is less critical of course, but it's good to be consistent.

I'll merge this branch with master now.

[sebastian-luna-valero]

I like it, many thanks!