phase viewer and ephemeris plugin #19

kecnry · 2023-05-17T18:36:49Z

This PR introduces a new ephemeris plugin with support for multiple ephemeris "components", which then creates and updates phase arrays and viewers which are kept up-to-date with the ephemeris and linked correctly to support subsets. This also includes a very simple interface to periodagrams which just propose the top peak - note that we will likely split periodograms/FFTs/PDM into their own plugins which interact with the ephemeris plugin, but this is at least useful for now.

Screen.Recording.2023-05-14.at.2.56.24.PM.mov

Possibly in scope:

components sometimes fail to be added/renamed
viewers occasionally crash (likely upstream bugs)

Intentionally deferred:

full periodogram support (input arguments, plot, interactivity)
ability to duplicate phase-viewers
aliasing support in phase-viewers
reference time incorrect when loading multiple light curves
prevent re-ordering of viewers when changing component name

* this does NOT yet actually compute/plot phases

* with just really rough estimates... we'll likely want to tweak the actual expressions

can test by manually forcing component options (until the UI is developed for adding components and UI/API for renaming/deleting): ``` ephem = lcviz.plugins['Ephemeris'] ephem.component._manual_options = ['default', 'new'] ephem.component.items = [{"label": opt} for opt in ['default', 'new']] ```

* NOTE: per-lightcurve reference times are not correct!

migrated from abandoned spacetelescope/jdaviz#2205

* export plot, for example, needs to be aware of viewer renames done via the ephemeris plugin

rosteen · 2023-05-22T21:32:52Z

Starting to test this, my initial reaction is that I like it other than one minor quibble and a possible bug. The quibble is that I'm not sure I like having the viewer automatically appear the first time you interact with the plugin. It seems a little arbitrary that I change, for example, the period, and suddenly the viewer appears. I wonder if it would be better to either have the default phase folded viewer be tabbed but unpopulated on load, or have a button to create the viewer.

The bug is that when the viewer first appears, the data isn't folded correctly (or at least isn't displaying correctly), and then fixes itself when I tweak any of the values slightly. In the screenshots below I first set the period to 5, and
then the second screenshot is after clicking the arrow to increment it up by .001:

kecnry · 2023-05-23T12:44:59Z

The quibble is that I'm not sure I like having the viewer automatically appear the first time you interact with the plugin. It seems a little arbitrary that I change, for example, the period, and suddenly the viewer appears. I wonder if it would be better to either have the default phase folded viewer be tabbed but unpopulated on load, or have a button to create the viewer.

We can easily iterate on this in the future as we get a feel for how users (including ourselves) use the tool. Since there will be so many different possible viewers, I thought it made sense to have them handled as dynamically as possible, and I can't think of any reason why you would want to change an ephemeris but not see the phased light curve. The button is already implemented though, so we could easily just remove one line of code to prevent the automatic creation on changing a traitlet.

The bug is that when the viewer first appears, the data isn't folded correctly (or at least isn't displaying correctly), and then fixes itself when I tweak any of the values slightly.

Is this with the data from the example notebook? It is possible that this is just aliasing based on the cadence of the data (period=0.42912 for example is a multiple of the cadence and looks similar), but I can't reproduce with that same data (HAT-P-11 Q10) and period of exactly 5... can you post a screen recording?

rosteen · 2023-05-23T14:23:23Z

This is for the example data, it's not an aliasing issue since it doesn't depend on what the initial period you put in, here you can see it happens if you initialize to either the Lomb-Scargle calculated value or to a user input. I forgot to show this, but if you go from the initial value of 5 to 5.001 and then back to 5, the plot looks fine with period of 5 the second time.

Screen.Recording.2023-05-23.at.10.20.17.AM.mov

kecnry · 2023-05-23T14:46:27Z

That is bizarre - I can't reproduce on my end. If you call the following when the phasing is incorrect (before updating anything to fix it otherwise), does it show the correct ephemeris and then update the plot correctly?

ephem = lcviz.plugins['Ephemeris']
print(ephem.ephemerides)
ephem._obj._update_all_phase_arrays()

rosteen · 2023-05-23T15:03:28Z

It prints the correct value but doesn't update the plot:

I update my glue-jupyter and bqplot installations just in case, didn't help. Maybe I should update glue-core as well.

kecnry · 2023-05-23T15:05:52Z

ok, then can I ask you to just try to parse _update_all_phase_arrays, add some debug print statements, and try to track down why it isn't actually updating the array (or maybe setting the links correctly)? 😬 The method should take the ephemeris (which is updated) and update the plot. It's very odd that it does once making another change 🤔

lcviz/plugins/ephemeris/ephemeris.py

* seems to fix a bug that can sometimes result in the phased-data showing incorrectly on the first change

rosteen

I didn't go over the code with too much rigor, but after the fix to the initial viewer population everything seems to work pretty well. The period finding doesn't seem very good, but I think that's an consequence of the example light curve being a mess with lots of stellar variability. We can definitely iterate from here.

bmorris3 · 2023-05-24T18:40:29Z

Are you expecting the warning shown here? (Using jdaviz on main.)

kecnry · 2023-05-24T18:45:49Z

jdaviz main will now require #20 and spacetelescope/jdaviz#2214 (or just use jdaviz before spacetelescope/jdaviz#2199)

bmorris3 · 2023-05-24T19:03:04Z

Just had to share – this works so beautifully for use cases on spotted, flaring stars. Thanks to @jradavenport for the rotation period measurement 👌🏻 .

bmorris3

This looks great. Feel free to merge after you've responded to the comments below (even if you don't adopt all suggestions; I just want to start these conversations).

lcviz/plugins/ephemeris/ephemeris.vue

bmorris3 · 2023-05-24T19:19:37Z

lcviz/plugins/ephemeris/ephemeris.vue

+    <lcviz-editable-select
+      :mode.sync="component_mode"
+      :edit_value.sync="component_edit_value"
+      :items="component_items"
+      :selected.sync="component_selected"
+      label="Component"
+      hint="Select an ephemeris component."
+      >
+    </lcviz-editable-select>


It isn't clear to me what this is meant to do, which might mean we need a better name/label. It seems like this controls part of the name of the ephemeris tab in the viewer, which is the viewer reference name.

I think you named it "Component" because it could be a "component" of the physical system, like planet b or c, or the rotation period of a primary or secondary "component" of a binary. Is that right? If so, maybe a better name for this box is "Ephemeris viewer label" or similar.

I struggled with this a bit as well, and think as we start to write other plugins it might become more clear and we might want to revise this. Elsewhere I think we'll want to refer to this label just as "ephemeris" - binning_plugin.ephemeris to bin the data phased by a specific ephemeris, for example - but that feels awkward here since I wanted to use the ephemeris property to return the actual ephemeris information. I do want to keep some synergy between the plugin attribute name (currently ephemeris_plugin.component) and the UI.

Understood. The crux of my concern is that the word "component" has too many meanings (glue Component? system component? viewer component?).

agreed (and especially with model components), please do bring this back up when we get more context with other plugins starting to fall into place and let's see if we can come up with a consistent but intuitive solution.

bmorris3 · 2023-05-24T19:26:39Z

lcviz/plugins/ephemeris/ephemeris.vue

+              <v-icon style="cursor: pointer" @click="period_halve">mdi-division</v-icon>
+            </j-tooltip>
+            <j-tooltip tooltipcontent="double period">
+              <v-icon style="cursor: pointer" @click="period_double">mdi-multiplication</v-icon>


Just wanted to add here that I had expressed early on that I might want something other than the mult/div buttons, but now that I've played with it this way, it's actually really clear that this is a handy and intuitive interface. Neat!

lcviz/plugins/ephemeris/ephemeris.vue

kecnry added 15 commits May 17, 2023 14:13

basic ephemeris plugin and dynamic phase-viewer creation

9bcd94e

* this does NOT yet actually compute/plot phases

groundwork for multiple component support

a02f370

ephemeris plugin: phase-folding and dpdt

3203c10

dynamic step-sizes

72ecdb1

* with just really rough estimates... we'll likely want to tweak the actual expressions

allow creating viewer through API and handle multiple LCs (in theory)

9d2c6ba

button to manually show phase viewer

5c96b2a

buttons to halve/double period

bc42eaf

support multiple light curves

fdf5dfb

* NOTE: per-lightcurve reference times are not correct!

basic plugin test coverage

f343ee2

rename_viewer support

928716f

migrated from abandoned spacetelescope/jdaviz#2205

add/rename/remove component support

cdb45cb

barebones periodogram implementation

64d7abe

prevent crash if viewer item doesn't exist

272d837

support calling update_ephemeris for non-active component

18f4c45

kecnry requested review from bmorris3, duytnguyendtn and rosteen May 17, 2023 18:36

kecnry added 3 commits May 17, 2023 14:44

fix codestyle

4f44296

monkeypatch ViewerSelect so jdaviz plugins use it

45a526f

* export plot, for example, needs to be aware of viewer renames done via the ephemeris plugin

update out-dated TODO comments

689ffb8

rosteen reviewed May 23, 2023

View reviewed changes

lcviz/plugins/ephemeris/ephemeris.py Outdated Show resolved Hide resolved

create viewer before phasing arrays

bc09338

* seems to fix a bug that can sometimes result in the phased-data showing incorrectly on the first change

rosteen approved these changes May 23, 2023

View reviewed changes

bmorris3 approved these changes May 24, 2023

View reviewed changes

label/hint tweaks

be48d7f

kecnry merged commit fb67575 into spacetelescope:main May 24, 2023

kecnry deleted the phase-viewer branch May 24, 2023 19:42

This was referenced May 31, 2023

enable coords-info and markers plugin #22

Merged

phasing incorrect immediately after phase-viewer creation #25

Closed

kecnry mentioned this pull request Jul 6, 2023

WIP: SubsetSelect support for renaming subsets spacetelescope/jdaviz#1291

Closed

9 tasks

bmorris3 mentioned this pull request Aug 3, 2023

Support for viewer reference name updates spacetelescope/jdaviz#2338

Merged

9 tasks

kecnry mentioned this pull request Aug 4, 2023

Footprints plugin spacetelescope/jdaviz#2341

Merged

20 tasks

kecnry mentioned this pull request Aug 14, 2023

Moving editable select upstream to jdaviz #36

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

phase viewer and ephemeris plugin #19

phase viewer and ephemeris plugin #19

kecnry commented May 17, 2023 •

edited

Loading

rosteen commented May 22, 2023

kecnry commented May 23, 2023

rosteen commented May 23, 2023

kecnry commented May 23, 2023

rosteen commented May 23, 2023

kecnry commented May 23, 2023

rosteen left a comment

bmorris3 commented May 24, 2023

kecnry commented May 24, 2023 •

edited

Loading

bmorris3 commented May 24, 2023

bmorris3 left a comment

bmorris3 May 24, 2023

kecnry May 24, 2023

bmorris3 May 24, 2023

kecnry May 24, 2023

bmorris3 May 24, 2023

phase viewer and ephemeris plugin #19

phase viewer and ephemeris plugin #19

Conversation

kecnry commented May 17, 2023 • edited Loading

rosteen commented May 22, 2023

kecnry commented May 23, 2023

rosteen commented May 23, 2023

kecnry commented May 23, 2023

rosteen commented May 23, 2023

kecnry commented May 23, 2023

rosteen left a comment

Choose a reason for hiding this comment

bmorris3 commented May 24, 2023

kecnry commented May 24, 2023 • edited Loading

bmorris3 commented May 24, 2023

bmorris3 left a comment

Choose a reason for hiding this comment

bmorris3 May 24, 2023

Choose a reason for hiding this comment

kecnry May 24, 2023

Choose a reason for hiding this comment

bmorris3 May 24, 2023

Choose a reason for hiding this comment

kecnry May 24, 2023

Choose a reason for hiding this comment

bmorris3 May 24, 2023

Choose a reason for hiding this comment

kecnry commented May 17, 2023 •

edited

Loading

kecnry commented May 24, 2023 •

edited

Loading