tardis icon indicating copy to clipboard operation
tardis copied to clipboard

Porting the line ID tool from tardisanalysis

Open jamesgillanders opened this issue 3 years ago • 12 comments

This PR moves the tardis_line_id tool across from tardisanalysis to the main TARDIS repository. The TARDIS line ID tool was originally written by @unoebauer.

Description Code has been updated to work with the newest version of TARDIS, but behaves as before.

The tool can be used to extract a list of the N most prominent transitions (default has N = 20) within a user-specified wavelength range (default is the entire wavelength range of the simulation). This can be particularly helpful for diagnosing the species with the largest contribution within a wavelength range, which perhaps has a blend of different absorption features.

Motivation and context This PR simply relocates the old tool, and makes it compatible with the new version of TARDIS.

How has this been tested?

  • [ ] Testing pipeline.
  • [x] Other. Ran the tardis_example.yml on the old and new versions. Output plots were similar.

Examples Here is the output plot from the old tool:

test_line_ID_K2.pdf

And here is the new version:

test_output.pdf

Type of change

  • [ ] Bug fix.
  • [x] New feature.
  • [ ] Breaking change.
  • [ ] None of the above.

Checklist

  • [x] My change requires a change to the documentation.
    • [ ] I have updated the documentation accordingly.
    • [ ] (optional) I have built the documentation on my fork following the instructions.
  • [x] I have assigned and requested two reviewers for this pull request.

jamesgillanders avatar Feb 07 '22 17:02 jamesgillanders

Before a pull request is accepted, it must meet the following criteria:

  • [ ] Is the necessary information provided?
  • [ ] Is this a duplicate PR?
    • [ ] If a new PR is clearly a duplicate, ask how this PR is different from the original PR?
    • [ ] If this PR is about to be merged, close the original PR with a link to this new PR that solved the issue.
  • [ ] Does it pass existing tests and are new tests provided if required?
    • [ ] The test coverage should not decrease, and for new features should be close to 100%.
  • [ ] Is the code tidy?
    • [ ] No unnecessary print lines or code comments.

tardis-bot avatar Feb 07 '22 17:02 tardis-bot

I moved this tool and the opacities.py tool to a folder titled 'analysis_tools' - that's why 2 new files are appearing instead of 1

jamesgillanders avatar Feb 07 '22 17:02 jamesgillanders

Codecov Report

Merging #1890 (d73f310) into master (c705946) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1890   +/-   ##
=======================================
  Coverage   62.55%   62.55%           
=======================================
  Files          68       68           
  Lines        7127     7127           
=======================================
  Hits         4458     4458           
  Misses       2669     2669           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c705946...d73f310. Read the comment docs.

codecov[bot] avatar Feb 07 '22 17:02 codecov[bot]

I will revisit this soon and try to get it to a merge-able state

jamesgillanders avatar Nov 10 '23 15:11 jamesgillanders

@jamesgillanders can this still be fixed up to merge?

andrewfullard avatar Apr 12 '24 14:04 andrewfullard