tomasstolker / species

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

Tutorial "Fitting data with a grid of model spectra" (and a few other pages): small things #79

Closed gabrielastro closed 7 months ago

gabrielastro commented 7 months ago

Following your request, I am submitting an "Issue" for several small things.

Not-major, not-minor things

  1. Unless I am mistaken, in https://species.readthedocs.io/en/latest/tutorials/fitting_model_spectra.html, there is missing at the top the line:

    from species.plot.plot_spectrum import plot_spectrum

    As is, ipython was not finding plot_spectrum() under "Plotting the spectral energy distribution"

  2. How does plot_spectrum know to evaluate the model at each sample in samples? modelbox is set only as = read_model.get_model(model_param=best, …, i.e. only with best. How does is a spec_res picked for the samples? This could be clarified.

Minor things

  1. At the top, about LD_LIBRARY_PATH: it might be cleaner (and/or less clean but easier for casual users) to set it in.bashrc or its Mac equivalent, and you might want to comment that the variable is with DYLD… for Macs but LD… under linux.

  2. "… be found in the API documentation of FitModel." The link gives Error 404.

  3. "run_multinest) and run_ultranest)" There are extra parentheses.

  4. The output of code block [8] has "Object: Unknown", which looks like a contradiction:

    Adding object: beta Pic b
    - GRAVITY spectrum:
       - Object: Unknown
    …
  5. "we will use the run_multinest method since UltraNest is somewhat more straightforward to install than MultiNest" It sounds like you meant "even though MultiNest is less straightforward to install"…

  6. Under "Plotting-the-posterior-samples": "Here we specify the database tag with the results from run_ultranest" You must mean multinest and you could comment that the tag can be a different name for each run (varying parameters or with ultranest or multinest)

  7. Is there an easy way (without digging into Matplotlib…) of not plotting the parallax row and column in the corner plot? It can be good to check but since it is an input, one probably often wants to skip it.

  8. In "Next, we calculate the residuals of the best-fit model with get_residuals", the link "get_residuals" has the wrong anchor as target:

    #species.util.phot_util.get_residuals
    →
    #species.util.fit_util.get_residuals
  9. On that page (https://species.readthedocs.io/en/latest/species.util.html#species.util.phot_util.get_residuals), by the way, there is an extra `:

    "Only required with spectrum='petitradtrans'`. Make sure that"
    →
    Only required with spectrum='petitradtrans'. Make sure that
  10. Back to the tutorial::

Now that we have gather all → Now that we have gathered all

  1. Suggestion: Add by default minor ticks and/or 1sigma and/or 3sigma lines in the residuals panel of the plotted spectrum. Especially when |ylim_res| is large (>3 sigma) is this useful: whether the points somewhere in the middle of the xrange have residuals of ~1 or ~5 sigma is somewhat important… I have seen now a few papers where this is tough to see.

  2. At https://species.readthedocs.io/en/latest/tutorials/emission_line.html: "subtract_continuum method of EmissionLine" contains two links that both give an Error 404. Maybe there is a function to detect links that are not valid anymore and/or to update links in the documentation and tutorial when the source code changes…

  3. At https://species.readthedocs.io/en/latest/overview.html, "Spectra of directly imaged planets and brown dwarfs" points to https://github.com/tomasstolker/species/blob/main/species/data/companion_data.json also gives an Error 404

  4. Same on another page (sorry, I do not know which one), with the text (I copied it but not the URL): "A library of magnitudes and parallaxes of directly imaged companions are available in the file." This contains a link giving an Error 404.

  5. From species/data/filter_data/filter_data.py:227:

    UserWarning: The minimum transmission value of Subaru/CIAO.z is smaller than zero (-1.80e-03).

    This is from the SVO but maybe you could fix it and/or ask them to?

  6. Overigens: https://github.com/tomasstolker/species/issues/70 is gesloten, maar is niet als zodanig gemarkeerd.

Thanks again for really a great tutorial! These are just small notes I made on the way.

tomasstolker commented 7 months ago

Thanks a lot Gabriel, this is really helpful! I have implemented most your suggestions in commit eb0f34e70ae75eecee7ed30def3debc1f90faa2a.

  1. This seems already explained when get_mcmc_spectra is used for generating the spectrum samples.

  2. There is a link to the installation instructions where that is explained.

  3. I have fixed the broken links in this tutorial, but not yet other pages (the structure of the package had changed recently...)

  4. This is a value read from the FITS file, so it wasn't stored there

  5. Should indeed have been multinest (also for 2.)

  6. Yes! You can check functionalities in the API docs

  7. It seems to include the minor ticks on my end, but I may have it on by default... Will add it to the actual function! The function returns the Figure object so this can also be adjusted manually.

  8. That is not the full warning I think? I should also mention that negative values are removed.

  9. I have closed that issue now 👍

Thanks again 🙏!

tomasstolker commented 7 months ago

I think that the minor ticks in the residuals panel from plot_spectrum are already on actually from what I could tell.

tomasstolker commented 7 months ago

In commit df05801d72de297d9ce773708a8e17e5be30902d I have added the unpack_tar parameter to add_model and also fixed most of the broken links in the docs (I think).

gabrielastro commented 7 months ago

I think that the minor ticks in the residuals panel from plot_spectrum are already on actually from what I could tell.

Ok, thanks for checking. At least for ±2 sigma or ±5 sigma, there are no minor ticks at my end.

gabrielastro commented 7 months ago

Sorry for missing the convenient plot_posterior options! Indeed, what I meant was actually available.

I did not mean to suggest that the negative values are not removed, just that the input data could be fixed "once and for all" :). Just from the approach of trying to minimise warnings to avoid drowning important ones…

Sorry for the mess-up in the numbering above, by the way. Thanks for all your changes!