Add ion contributions to SDEC plotter #1539
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The elements coloured in the plot are determined via the absorption and emission. It's possible however, that these elements are not the same, e.g. if you specify a limited wavelength range. This could give an error if you try to plot an element not in the emission dataframe, for example. This adds a try statement to catch this case
Added a new column to the packet dataframe that gives the species id. This combines the atomic and ion numbers into a unique id that will be used to group species later.
Added parser to convert request species list into ids to be used by plotter
Call species list parser function
Rearranged parser so it didn't crash if species_list was none
Added in the ion contribution stuff for the mpl plotting. Also added a new function to get the colorbar labels. This is necessary to determine how many unique colours there should be
Added colouring for ion contributions in plotly figures
Fixed bug in the absorption and emission df calculations. Now they elements should be handled correctly so there won't be errors if a species appears in emission and not absorption, and vice versa Added comments throughout
MarkMageeAstro
requested review from
jaladh-singhal,
jamesgillanders and
marxwillia
Codecov Report
@@ Coverage Diff @@
## master #1539 +/- ##
==========================================
- Coverage 69.06% 67.92% -1.14%
==========================================
Files 68 68
Lines 6106 6217 +111
==========================================
+ Hits 4217 4223 +6
- Misses 1889 1994 +105
Continue to review full report at Codecov.
|
|
Why is Si II repeated twice in the colourbar of the example? |
It isn't. There's Si II, Si III, S II, and Ca. |
marxwillia
reviewed
Apr 16, 2021
There was a problem hiding this comment.
this is a really nicely written and well-presented bit of code. I have a few tests I want to finish before I approve the PR, but see comments below for minor corrections. As discussed in the meeting last week, if parts of the plotting code could be grouped into some form of function to reduce duplication, that would be amazing
jamesgillanders
approved these changes
Apr 19, 2021
Added a new function to work out what colours will be plotted. This was originally repeated multiple times in the code, but now it's a single function that's called multiple times Also fixed a bug in the absorption luminosities dataframe
Change name of some variables and fixed some typos.
jaladh-singhal
requested changes
Apr 22, 2021
There was a problem hiding this comment.
This looks much better than your initial version @MarkMageeAstro - excellent job in restructuring the code in functions. 🎉
I've few comments that shouldn't take much time to be addressed.
marxwillia
approved these changes
Apr 22, 2021
jaladh-singhal
requested changes
Apr 23, 2021
jaladh-singhal
approved these changes
Apr 23, 2021
Merged
10 tasks
atharva-2001
pushed a commit
to atharva-2001/tardis
that referenced
this pull request
Oct 1, 2021
* Add catch for missing elements The elements coloured in the plot are determined via the absorption and emission. It's possible however, that these elements are not the same, e.g. if you specify a limited wavelength range. This could give an error if you try to plot an element not in the emission dataframe, for example. This adds a try statement to catch this case * Fixed typo * Black formatting * Fixed typo * Add species column to packet df Added a new column to the packet dataframe that gives the species id. This combines the atomic and ion numbers into a unique id that will be used to group species later. * Added species list parser Added parser to convert request species list into ids to be used by plotter * Implement parser call Call species list parser function * Fixed parser Rearranged parser so it didn't crash if species_list was none * Implement MPL ion contributions Added in the ion contribution stuff for the mpl plotting. Also added a new function to get the colorbar labels. This is necessary to determine how many unique colours there should be * Changed order of functions * Implement plotly ion contributions Added colouring for ion contributions in plotly figures * Remove print statements * Fixed df bug and added comments Fixed bug in the absorption and emission df calculations. Now they elements should be handled correctly so there won't be errors if a species appears in emission and not absorption, and vice versa Added comments throughout * Black formatting * Added colour function Added a new function to work out what colours will be plotted. This was originally repeated multiple times in the code, but now it's a single function that's called multiple times Also fixed a bug in the absorption luminosities dataframe * Address review comments Change name of some variables and fixed some typos. * Change print to logger * Black formatting * Updated variable names and string formatting * Black formatting
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a port of existing code in TARDIS analysis. SDEC now has the ability to plot individual ions or elements that are requested by the user.
Description
This PR adds a new argument for the SDEC plotter: species_list. This is a list of ions, elements, or both provided by the user. The list is read and parsed by the code, and compared against what species are present in the model. Each requested species that is also in the model is given a unique colour in the plot.
species_list can take arguments as either individual ions, elements, a range of ions, or any combination of these. For example species_list = ['Si II', 'S I-V', 'Ca']. In this example, Si II interacting packets will be plotted as a single colour. All packets that interacted with any ion of Ca will be plotted as a single colour. Finally, the code will determine which ions from S I-V are present in the model and give each of those their own colour.
Motivation and context
Brings back existing functionality in TARDIS analysis.
How has this been tested?
Comparisons with older versions of the code and packets in model.
Examples
The following plot was generated using:
plotter.generate_plot_mpl(packet_wvl_range=[3000, 9000] * u.AA, species_list=['Si I-III', 'S II', 'Ca']);
Type of change
Checklist