-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add option to calculate higher-order moments in velocity #2584
Changes from 12 commits
b487954
3b56894
40f84ae
1563cb7
372d515
cdce7e7
79511eb
4ede319
43cbf70
8248953
b3486dd
ebc7a2f
e23aacf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,18 +2,21 @@ | |
from pathlib import Path | ||
|
||
from astropy import units as u | ||
from astropy.constants import c | ||
from astropy.nddata import CCDData | ||
import numpy as np | ||
|
||
from traitlets import Unicode, Bool, observe | ||
from traitlets import Bool, List, Unicode, observe | ||
from specutils import manipulation, analysis | ||
|
||
from jdaviz.core.custom_traitlets import IntHandleEmpty | ||
from jdaviz.core.custom_traitlets import IntHandleEmpty, FloatHandleEmpty | ||
from jdaviz.core.events import SnackbarMessage | ||
from jdaviz.core.registries import tray_registry | ||
from jdaviz.core.template_mixin import (PluginTemplateMixin, | ||
DatasetSelectMixin, | ||
SpectralSubsetSelectMixin, | ||
AddResultsMixin, | ||
SelectPluginComponent, | ||
with_spinner) | ||
from jdaviz.core.user_api import PluginUserApi | ||
|
||
|
@@ -42,6 +45,10 @@ class MomentMap(PluginTemplateMixin, DatasetSelectMixin, SpectralSubsetSelectMix | |
* ``spectral_subset`` (:class:`~jdaviz.core.template_mixin.SubsetSelect`): | ||
Subset to use for the line, or ``Entire Spectrum``. | ||
* ``n_moment`` | ||
* ``output_unit`` | ||
Choice of "Wavelength" or "Velocity", applicable for n_moment >= 1. | ||
* ``reference_wavelength`` | ||
Reference wavelength for conversion of output to velocity units. | ||
* ``add_results`` (:class:`~jdaviz.core.template_mixin.AddResults`) | ||
* :meth:`calculate_moment` | ||
""" | ||
|
@@ -51,6 +58,10 @@ class MomentMap(PluginTemplateMixin, DatasetSelectMixin, SpectralSubsetSelectMix | |
filename = Unicode().tag(sync=True) | ||
moment_available = Bool(False).tag(sync=True) | ||
overwrite_warn = Bool(False).tag(sync=True) | ||
output_unit_items = List().tag(sync=True) | ||
output_unit_selected = Unicode().tag(sync=True) | ||
reference_wavelength = FloatHandleEmpty().tag(sync=True) | ||
dataset_spectral_unit = Unicode().tag(sync=True) | ||
|
||
# export_enabled controls whether saving the moment map to a file is enabled via the UI. This | ||
# is a temporary measure to allow server-installations to disable saving server-side until | ||
|
@@ -62,6 +73,11 @@ def __init__(self, *args, **kwargs): | |
|
||
self.moment = None | ||
|
||
self.output_unit = SelectPluginComponent(self, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we want this - and probably There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call, I can do that after tagup. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
items='output_unit_items', | ||
selected='output_unit_selected', | ||
manual_options=['Wavelength', 'Velocity']) | ||
|
||
self.dataset.add_filter('is_cube') | ||
self.add_results.viewer.filters = ['is_image_viewer'] | ||
|
||
|
@@ -83,6 +99,7 @@ def user_api(self): | |
# NOTE: leaving save_as_fits out for now - we may want a more general API to do that | ||
# accross all plugins at some point | ||
return PluginUserApi(self, expose=('dataset', 'spectral_subset', 'n_moment', | ||
'output_unit', 'reference_wavelength', | ||
'add_results', 'calculate_moment')) | ||
|
||
@observe("dataset_selected", "dataset_items", "n_moment") | ||
|
@@ -93,6 +110,16 @@ def _set_default_results_label(self, event={}): | |
label_comps += [f"moment {self.n_moment}"] | ||
self.results_label_default = " ".join(label_comps) | ||
|
||
@observe("dataset_selected") | ||
def _set_data_units(self, event={}): | ||
if self.dataset_selected != "": | ||
# Spectral axis is first in this list | ||
if self.app.data_collection[self.dataset_selected].coords is not None: | ||
unit = self.app.data_collection[self.dataset_selected].coords.world_axis_units[0] | ||
self.dataset_spectral_unit = unit | ||
else: | ||
self.dataset_spectral_unit = "" | ||
|
||
@with_spinner() | ||
def calculate_moment(self, add_data=True): | ||
""" | ||
|
@@ -126,6 +153,17 @@ def calculate_moment(self, add_data=True): | |
if data_wcs: | ||
data_wcs = data_wcs.swapaxes(0, 1) # We also transpose WCS to match. | ||
self.moment = CCDData(analysis.moment(slab, order=n_moment).T, wcs=data_wcs) | ||
if n_moment > 0 and self.output_unit_selected.lower() == "velocity": | ||
# Catch this if called from API | ||
if not self.reference_wavelength > 0.0: | ||
raise ValueError("reference_wavelength must be set for output in velocity units.") | ||
power_unit = f"{self.dataset_spectral_unit}{self.n_moment}" | ||
self.moment = np.power(self.moment.convert_unit_to(power_unit), 1/self.n_moment) | ||
self.moment = self.moment << u.Unit(self.dataset_spectral_unit) | ||
ref_wavelength = self.reference_wavelength * u.Unit(self.dataset_spectral_unit) | ||
relative_wavelength = (self.moment-ref_wavelength)/ref_wavelength | ||
in_velocity = np.power(c*relative_wavelength, self.n_moment) | ||
self.moment = CCDData(in_velocity, wcs=data_wcs) | ||
|
||
fname_label = self.dataset_selected.replace("[", "_").replace("]", "") | ||
self.filename = f"moment{n_moment}_{fname_label}.fits" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,7 @@ | |
hint="Select the data set." | ||
/> | ||
|
||
<plugin-subset-select | ||
<plugin-subset-select | ||
:items="spectral_subset_items" | ||
:selected.sync="spectral_subset_selected" | ||
:has_subregions="spectral_subset_selected_has_subregions" | ||
|
@@ -35,6 +35,37 @@ | |
></v-text-field> | ||
</v-row> | ||
|
||
<div v-if="n_moment > 0 && dataset_spectral_unit !== ''"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a little torn by the fact that this isn't checked for in the API at all, but I think that's probably ok since the idea is that the plugin APIs just let you modify inputs. Something like #2347 would address this concern in general. The other option would be to have changing Definitely doesn't need to hold up approval/merge, just wanted to leave a papertrail of thoughts. |
||
<v-row> | ||
<v-radio-group | ||
label="Output Units" | ||
hint="Choose whether calculated moment is in units of wavelength or velocity." | ||
v-model="output_unit_selected" | ||
column | ||
class="no-hint"> | ||
<v-radio | ||
v-for="item in output_unit_items" | ||
:key="item.label" | ||
:label="item.label" | ||
:value="item.label" | ||
></v-radio> | ||
</v-radio-group> | ||
</v-row> | ||
<v-row v-if="output_unit_selected === 'Velocity'"> | ||
<v-text-field | ||
ref="reference_wavelength" | ||
type="number" | ||
label="Reference Wavelength" | ||
v-model.number="reference_wavelength" | ||
:suffix="dataset_spectral_unit.replace('Angstrom', 'A')" | ||
hint="Rest wavelength of the line of interest" | ||
persistent-hint | ||
:rules="[() => reference_wavelength !== '' || 'This field is required', | ||
() => reference_wavelength > 0 || 'Wavelength must be positive']" | ||
></v-text-field> | ||
</v-row> | ||
</div> | ||
|
||
<plugin-add-results | ||
:label.sync="results_label" | ||
:label_default="results_label_default" | ||
|
@@ -49,7 +80,7 @@ | |
:action_spinner="spinner" | ||
@click:action="calculate_moment" | ||
></plugin-add-results> | ||
|
||
<j-plugin-section-header v-if="export_enabled">Results</j-plugin-section-header> | ||
|
||
<div style="display: grid; position: relative"> <!-- overlay container --> | ||
|
@@ -80,10 +111,10 @@ | |
opacity=1.0 | ||
:value="overwrite_warn && export_enabled" | ||
:zIndex=3 | ||
style="grid-area: 1/1; | ||
style="grid-area: 1/1; | ||
margin-left: -24px; | ||
margin-right: -24px"> | ||
|
||
<v-card color="transparent" elevation=0 > | ||
<v-card-text width="100%"> | ||
<div class="white--text"> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this defaults to zero, but since its the original value, does not result in a validation error on the input, and ultimately results in a traceback (to avoid infinites in the velocities) 🤔
We could have this update to the central value of the selected data/subset.. but then that could override a user-provided value unless we had some logic like we do for labels. I think our best bet for now is to validate the inputs at the "calculate" button-level (probably can be done on the vue-side since it just needs
reference_wavelength
,output_unit_selected
andn_moment
) and disable the button with explanation text (we usually use<span class="v-messages v-messages__message text--secondary" style="color: red !important">
for situations like this elsewhere).