tomasstolker / species

Toolkit for atmospheric characterization of directly imaged exoplanets
https://species.readthedocs.io
MIT License
22 stars 9 forks source link

Various small things #86

Closed gabrielastro closed 8 months ago

gabrielastro commented 9 months ago
  1. At start-up, doing SpeciesInit() leads to:

    #UserWarning: no type annotations present -- not typechecking species.util.dust_util.check_dust_database
  2. Would it be good to convert silently resolution in database.add_object() to a float? Currently:

    type of argument "spec_res" must be one of (float, NoneType); got numpy.int64 instead

    Keeping resolution an integer makes it easy to combine it with filenames, for instance, and one often thinks of integer resolution.

  3. The documentation says about add_object(): Function for adding the photometry and/or spectra of an object to the database and the tutorial writes: "The use of add_object is incremental so when rerunning the method it will add new data in case a filter or spectrum name hadn’t been used before, or it will overwrite existing data in case a filter or spectrum name does already exist" but actually it replaces the data when adding spectral data in two different calls:

    database.add_object(object_name = 'myPMO', parallax=(100,0.1), spectrum = {'onepart': ('file1.dat', None, 1000)})
    database.add_object(object_name = 'myPMO', parallax=(100,0.1), spectrum = {'anotherpart': ('file2.dat', None, 1000)})

    After the second call, database.get_object('myPMO').open_box() will only list the 'anotherpart' spectral data. If the parallax is not set in the second call, it does keep it from the first (as should be).

Honestly, I have found it up to now practical to have the data overwritten and then in FitModel not have to worry about (because I forgot to…) including only the datasets I want, but at other times it can lead to unpleasant surprises. Maybe provide an overwritedata= (by default False) keyword?

  1. Two tiny things in species/util/box_util.py: " but the argument of 'model' is set to ""but the argument of 'model' is set to " :nerd_face:

"is differently implemented with" → "is implemented differently with"

  1. When calling database.list_content(), it would be nice to print the stored resolution of the spectral data.

  2. Similarly, it would be useful to output the (lowest, average, highest) resolution in the model when doing modelbox_full.open_box()

  3. In ./species/plot/plot_mcmc.py:

    print(f"Plotting the posterior: {output}...", end="", flush=True)
    →
    print(f"Plotting the posterior: {output} ...", end="", flush=True)

    This would make copying the filename from the terminal easier: without a space, the three dots get selected when double clicking on the name :D.

  4. In corner plot, for the column title at least for ATMO: ad_index = {} → $\gamma = {}$

  5. Put the (e.g.) "1e-16" as "10^{-16}" into the parenthesis with the units of flux density (classical way) instead of on top of the plot (defensible but less elegant and takes up one whole line for a number that is not of detailed interest)

  6. When in plot_spectrum units gets implemented, or until then as an option, to have erg/cm²/s/Å units for the flux density (could be called "cgs-like" or "Swedish-style" (svensk=True?) because of Å :wink:)

Thanks!

tomasstolker commented 8 months ago

Hi Gabriel! I think this has all been implemented in the meanwhile? For 10, you can use the new units parameter in plot_spectrum. For 9, this will get adjusted I think when setting the ylim parameter. I have added the spec_res attribute to the ModelBox in the latest commit f1f6807fc78669f7b9bc4d9a7e184a35c8a83a69. I hope this helps!

gabrielastro commented 8 months ago

Hello Tomas! Very nice—thanks a lot!

tomasstolker commented 8 months ago

Done! The sampling can be obtained with the get_sampling() method from ReadModel.

gabrielastro commented 8 months ago

Thanks :grin:! Sorry, last tweaks ("sigh once, give lots of users even more comfort"):

gabrielastro commented 8 months ago

Also, sorry but when using units=("µm","erg s-1 cm-2 angstrom-1"), there must be a factor of cm/µm=1e-4 dividing instead of multiplying in the flux density because I plot some data with "W m-2 µm-1" and get F_lambda = 1e-16 W/m²/µm but supposedly F_lambda = 1e-9 erg/s/cm²/Å instead of 1e-17 erg/s/cm²/Å; the difference is 1e-8 = (1e-4)² :smile:… a classic.

tomasstolker commented 8 months ago

Ah! My bad, I thought the conversion from cm-2 and A-1 cancelled out. This has been fixed now 👍, so we can close this issue.

gabrielastro commented 8 months ago

Excellent! Thank you very much. While marking this as closed, I would just suggest in the species.util.data_util.convert_units docstring:

…
"Jy", "MJy"). One can use "um" or "µm" interchangeably, and similarly "AA", "Å", "A", or "angstrom".