Short comment on A.1

[ElBardo99]

Tutorial A.1 fetches data creating a new directory

workdir = cwd + '/tuto_A.1/'
os.makedirs(workdir, exist_ok=True)

If the whole tutorial is not completed within the same session, future sessions show an error at:

fetch.fetch_open_samples(evt, catalog=catalog, 
                        unpack=True, read_file=False, delete_on_exit=False, 
                        outdir=workdir)

However, this error is harmless, since one can go on to the next cell and complete the tutorial. However, even if now the tutorial is completed, the error is displayed for future sessions. Only solution at this point is ''' rm -rf tuto_A.1 ''' from shell. People using the tutorials should be aware of this.

Moreover, it is not clearly explained how to deal with (pasting here the original, unmodified version):

env_path = '/cvmfs/oasis.opensciencegrid.org/ligo/sw/conda/envs/igwn-py38/'

# Here we use the following command to find out the path to the binary file of the command "lalinference_nest":
! which lalinference_nest
# Then we specify the correct path:
env_path = '/home/jerry/miniforge3/envs/odw-2024-test/'

In fact ! which lalinference_nest gives: /path/you/actually/need/bin/lalinference_nest , and one should have: env_path = /path/you/actually/need/ . To new users, it is not 100% clear this is actually what the code needs to work IMO.

[martinberoiz]

(Without looking at the code) could a solution be:

env_path="`which lalinference_nest`/../.."

[duboism]

(Without looking at the code) could a solution be:

env_path="`which lalinference_nest`/../.."

I have tested this and I think this code doesn't work: AFAIK it's shell and ipython doesn't allow this. With ipython you can do something like env_path = ! which lalinference_nest but then one has to walk up the directories.

There is a pure python solution:

import pathlib, shutil
env_path = pathlib.Path(shutil.which("lalinference_nest")).parent.parent

A bit verbose.

I have looked a bit at the code: env_path is used to forge the path for a few executables (lalinference_nest, lalinference_mpi_wrapper, mpi_run and lalinference_mcmc) assuming they are all in the same directory env_path + 'bin/' (this is OK for those notebooks but not generally). So I fact, I think it's even better to get rid of this variable and just use shutil.which for every program: this is more general, is proper python and clarify the notebook (and easy to do).

[duboism]

I implemented the approach described above in branch which_which. AFAIK, the generated bash file (lalinference_run.sh) is identical.

[duboism]

I implemented the approach described above in branch which_which.

If nobody is opposed, I will merge this. @ElBardo99, @agoodmanjerry, any comment ?

[ElBardo99]

My only suggestion would be that of adding: - nodefaults in channels in the environment.yml, just to speed up installation (not necessary, anyway). All good IMO, Giuseppe Inviato da Outlook per Android<https://aka.ms/AAb9ysg>

…

________________________________ From: Mathieu Dubois ***@***.***> Sent: Tuesday, April 16, 2024 10:06:19 AM To: gw-odw/odw-2024 ***@***.***> Cc: ElBardo99 ***@***.***>; Mention ***@***.***> Subject: Re: [gw-odw/odw-2024] Short comment on A.1 (Issue #10) I implemented the approach described above in branch which_which<https://github.com/gw-odw/odw-2024/tree/which_which>. If nobody is opposed, I will merge this. @ElBardo99<https://github.com/ElBardo99>, @agoodmanjerry<https://github.com/agoodmanjerry>, any comment ? — Reply to this email directly, view it on GitHub<#10 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BH24FWFJMD2O4CFFLX5YIVDY5TLXXAVCNFSM6AAAAABGHP7Z62VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJYGQ4DMNBRGY>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[duboism]

My only suggestion would be that of adding: - nodefaults in channels in the environment.yml, just to speed up installation (not necessary, anyway).

I think you are talking about the other issue (updating the environment), right ?

[ElBardo99]

My God, sorry. I misread the subject of the mail. BTW, seems all good on the A.1 issue, as far as I can see ( on smartphone now). Sorry again, Giuseppe Inviato da Outlook per Android<https://aka.ms/AAb9ysg>

…

________________________________ From: Mathieu Dubois ***@***.***> Sent: Tuesday, April 16, 2024 10:14:40 AM To: gw-odw/odw-2024 ***@***.***> Cc: ElBardo99 ***@***.***>; Mention ***@***.***> Subject: Re: [gw-odw/odw-2024] Short comment on A.1 (Issue #10) My only suggestion would be that of adding: - nodefaults in channels in the environment.yml, just to speed up installation (not necessary, anyway). I think you are talking about the other issue (updating the environment), right ? — Reply to this email directly, view it on GitHub<#10 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/BH24FWBVISPDSXYCQN46KVDY5TMXBAVCNFSM6AAAAABGHP7Z62VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJYGUYDGOBQG4>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[camurria]

My only suggestion would be that of adding: - nodefaults in channels in the environment.yml, just to speed up installation (not necessary, anyway). All good IMO, Giuseppe Inviato da Outlook per Androidhttps://aka.ms/AAb9ysg
…

I will add - nodefaults in the updated version of environment

[duboism]

OK, I have merged the branch (see #11).

[ElBardo99]

Hello @duboism , I've just tested the

env_path = pathlib.Path(shutil.which("lalinference_nest")).parent.parent

and it still causes an error, namely because of a missed "/" at the end.

This one works without changing other cells:

import pathlib, shutil
env_path = ''.join((str(pathlib.Path(shutil.which("lalinference_nest")).parent.parent), '/'))

[duboism]

You're right, I haven't tested the whole notebook based on this approach. However, it's no longer necessary after #11.

[duboism]

Here are my thought on the other problem you raised (re-running the notebook fails because some files are left):

• it's good to be able to re-run the notebook without failure
• implementing a true error recovery system is not trivial and we probably don't have time
• deleting the directory when running the notebook "works" but is a bit harsh (maybe the user modified some things in the directory)
• you can delete directories from Jupyter Notebooks interface

Therefore, I propose a middle ground: we check if the directory exists before trying to create it; if so, we stop with the clearest (possible) error message, inviting people to save valuable data and delete the folder (and maybe providing a simple commented-out line to do that).