Trying to run sail example

[steven-murray]

Part of JOSS review:

I am trying to follow the examples on the README. The sail example gave me an error:

>>> sail.ldc_calculate('examples/sail_example1.txt`)
WARNING: /Users/pepe/Desktop/exotethys_versions/ExoTETHyS-master/Wavelength_bins_files/WFC3_G141_bins_Tsiaras2018_low.txt file not found.
WARNING: Ignoring wavelength_bins_file WFC3_G141_bins_Tsiaras2018_low.txt .
WARNING: assuming star_metallicity=0.0 for all targets. No input values were provided.
Downloading...  teff06000_logg4.0_MH0.0.pickle
Downloading...  teff06100_logg4.0_MH0.0.pickle
Downloading...  teff06000_logg4.5_MH0.0.pickle
Downloading...  teff06100_logg4.5_MH0.0.pickle
^[[OERROR: No legal passbands to calculate.

It seems that there is a hard-coded filepath in there?

[gmorello]

Yes, as many input files will be provided by the user it is normal to have hard-coded paths in the input files. However, the sail_example2.txt, sail_example5.txt, sail_example6.txt and sail_example7.txt should work without any edits.

[steven-murray]

OK, I think I see. Perhaps it would be better to have an example on the README that worked out of the box without user input, or at least say that the file needs to be changed before running the example.

[williamjameshandley]

I also get a similar issue when trying to run the suggested example in the README:

>>> from exotethys import trip
>>> trip.trip_calculate('examples/trip_example.txt')

WARNING: /Users/pepe/Desktop/aux_files/ld_Teff6100.0_logg4.5_MH0.0_TESS.txt file not found.
ERROR: invalid format for input_limb_file /Users/pepe/Desktop/aux_files/ld_Teff6100.0_logg4.5_MH0.0_TESS.txt . It must have 2 columns.
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
<ipython-input-2-3c574cbd2807> in <module>
----> 1 trip.trip_calculate('examples/trip_example.txt')

~/Projects/forks/ExoTETHyS/venv/lib/python3.8/site-packages/exotethys/trip.py in trip_calculate(configuration_file)
    952     [check, input_dict] = check_configuration(input_dict1) #Checking configuration file and dictionary update
    953     if check:
--> 954         process_configuration(input_dict) #Computing and saving
    955     else:
    956         print('Something went wrong with the input.')

~/Projects/forks/ExoTETHyS/venv/lib/python3.8/site-packages/exotethys/trip.py in process_configuration(input_dict)
    807     input_limb_path = input_dict_local['input_limb_path'][0]
    808     input_limb_file = input_dict_local['input_limb_file'][0]
--> 809     [mu_model_orig, radi_model_orig, intensity_model_orig] = get_input_limb_model(input_limb_type,input_limb_path,input_limb_file)
    810 
    811     cutting_limb = input_dict_local['cutting_limb'][0]

~/Projects/forks/ExoTETHyS/venv/lib/python3.8/site-packages/exotethys/trip.py in get_input_limb_model(input_limb_type, input_limb_path, input_limb_file)
    711     if not check_2Darray(limb_model, n_col=2):
    712         print('ERROR: invalid format for input_limb_file', input_limb_path+input_limb_file, '. It must have 2 columns.')
--> 713         exit()
    714     intensity_model = copy.deepcopy(limb_model[:,1])
    715     if np.min(intensity_model)<0.0:

NameError: name 'exit' is not defined

[gmorello]

It looks my previous answer got lost: now all the examples should run without errors if following the instructions of the README.

[williamjameshandley]

I currently get the error

>>> from exotethys import trip
>>> trip.trip_calculate('examples/trip_example.txt')
WARNING: ExoTETHyS-master/aux_files/ld_Teff6100.0_logg4.5_MH0.0_TESS.txt file not found.
ERROR: invalid format for input_limb_file ExoTETHyS-master/aux_files/ld_Teff6100.0_logg4.5_MH0.0_TESS.txt . It must have 2 columns.

[williamjameshandley]

However, if I rename the repository as ExoTETHyS-master, and then run the command from one directory above then it does run. Do you think it's possible to make the directories relative?

This command also seems to consume a lot of memory >10GB. Is this usual?

[gmorello]

About the error it depends on the name of the folder as @williamjameshandley noted. The way I download the repository it is automatically named ExoTETHyS-master, but apparently it is not always the case. This problem would not exist if I suggest running the code from the root directory. I proposed running it from one level above to actually test the installation, otherwise python would import the local directory called exotethys. However, given that the user is not expected to modify the source code, this is probably an unnecessary precaution. Would you prefer that I propose running the examples from the root directory?

[gmorello]

TRIP does consume a lot of memory as by default it uses 10^5 interpolated points for calculating integrals. (We are working on some optimization, but it will take some time because the optimal sampling is case-dependent).

[williamjameshandley]

TRIP does consume a lot of memory as by default it uses 10^5 interpolated points for calculating integrals. (We are working on some optimization, but it will take some time because the optimal sampling is case-dependent).

It might be worth putting a warning in the README, or finding an example which consumes a fraction of that amount of memory

[williamjameshandley]

About the error it depends on the name of the folder as @williamjameshandley noted. The way I download the repository it is automatically named ExoTETHyS-master, but apparently it is not always the case. This problem would not exist if I suggest running the code from the root directory. I proposed running it from one level above to actually test the installation, otherwise python would import the local directory called exotethys. However, given that the user is not expected to modify the source code, this is probably an unnecessary precaution. Would you prefer that I propose running the examples from the root directory?

I reckon it's probably more intuitive to run from within the directory, although I do acknowledge that this can cause import clashes.

[gmorello]

Ok, I modified the examples to be run from the root folder. Also changed the README and including the note about memory for the TRIP example. Does it work now?

[williamjameshandley]

Does it work now?

Yes : )

Also changed the README and including the note about memory for the TRIP example.

This note should be directly below/above the second example in the list of subpackages section

Once that's done I'm happy for this issue to be closed.

[steven-murray]

I strongly feel that the examples should be runnable from wherever you are, as long as you pass in the path to the example file. It should not be dependent on being in any given folder.

[gmorello]

I strongly feel that the examples should be runnable from wherever you are, as long as you pass in the path to the example file. It should not be dependent on being in any given folder.

Yes, but the correct path must be written in the example file.

[gmorello]

Or maybe I did not fully get it. The examples are simple text files, I do not want anything more complicated than that. As they may contain the path to read other files, either the absolute path or the relative path from where you are running the code must be specified in the example file.

[steven-murray]

Three things:

1. YAML or TOML files are almost certainly going to serve you better than the .txt files you have -- there are well-used packages to read them robustly.
2. The paths in the config files should be able to be either absolute or relative. If relative, they should be relative to the config file's location. In your examples, they should be relative, since you know the path relative to the file. This can easily be achieved using the os.path module.
3. You may want to consider installing the example data (not the example configs, but the data they use) along with the package, in which case the example data could be auto-loaded based on an import of the package. This is definitely optional, but worth considering.

[gmorello]

@steven-murray Thanks for your new comments.

1. For now I prefer using file formats that are more known by common users, unless it is strictly necessary or there is a huge advantage in using other formats. I will look what these formats are though.
2. Similarly, for this point, when I download a package and try to execute it without changing settings, I like that files are saved in the local folder which contains the actual work, rather than in the root folder, which I prefer to keep as the original. So, the path can already be relative, but from the working directory. Can we just let this as it is for now? If I will find out that the majority of users prefer a different setting, I will modify it later.
3. I have seen some packages doing this trick, but with the current setup if the user follows the instructions should be able to get to the example results without any trouble. After that, users will want to specify their paths anyway. So, I do not see an advantage in this particular case.

Last comment: the AJ paper was accepted 1 week ago, and they are only waiting for the link to the static version on Zenodo to publish it. I do not feel it is the case to futher delay the publication for this issue.

[steven-murray]

@gmorello : you're right that it currently works for the example on the README. I was confused because of all the absolute paths in the sail_example1.txt file, but it seems that they're all commented out anyway. This could perhaps be cleaned up a bit -- users don't need to know your directory structure ;-).

I'm OK with this going forward and not being a hindrance to publication. However, I'd strongly suggest considering my suggestions further in the near future.

[gmorello]

Examples cleaned, thanks. I am leaving the commented keywords anyway even if they are not used, just for convenience. I will consider the most recent suggestions for version 2 (this is going to be 1.0.2).

Soon I will re-check one last time that all the examples work as expected, then proceed with uploading to Pypi and Zenodo.