Updates for xija_gui_fit #116

jzuhone · 2022-02-03T14:07:02Z

Description

This PR fixes a number of annoying bugs in xija_gui_fit and refactors the design of a few features.

The console access has been removed. It was an experimental feature that never worked very well anyway, and getting it to work to design scripting of the interface was proving intractable.
Improved the handling of "bad times" which one wants to store in the JSON file and the "mask times", which includes the bad times but also includes times that are masked out temporarily in the GUI for fitting.
Added a splitter between the plots window and the parameter panel so you can adjust the horizontal size of the two parts
Fixed some issues with the scaling of the x-axis of plots, especially when adding/deleting. This was partially achieved by adding a dummy Figure, Axes pair that is never displayed but also never goes away and is used as the shared axes.
The command entry box, used for freezing and thawing parameters as well as adding times to mask and removing the mask times, has been removed, and the functionalities have been separated out. Now, freeze and thaw entry boxes appear above the parameters panel. A new button for "filters" appears on the left panel (see image below), which handles adding mask times. To freeze or thaw a parameter, enter the regular expression into either box (e.g., solarheat*__P*) and hit Enter.
General reorganization of the buttons on the left panel.
The ability to add a "bad time" to the model JSON file from the GUI has been added and is part of the "filters" panel.

The new look of the window:

The new "Filters" subwindow, showing how to add a mask and ignore all masks, and how to add a "bad time":

Testing

Passes unit tests on MacOS, linux, Windows (at least one required)
Functional testing

…e adds later

…tice to different elements. Fix some bugs.

…e properly

jzuhone · 2022-02-03T14:40:09Z

@taldcroft likely the only FSDS-relevant bits are in https://github.com/sot/xija/pull/116/files#diff-78ad09ad6fe546ab9d4b1d835063650634f425cff3b09df8417a13be438d6838

taldcroft · 2022-02-03T15:12:45Z

@jzuhone - awesome work glad to see this tool getting continued attention.

I think this will fall in the category of a package update that tags along in a normal Ska FSDS request. It wasn't obvious in 5 seconds what is going on with the diff you highlighted above, but from FSDS perspective the key is to highlight interface impacts at the top level (in the PR description) and make sure in advance that likely users are aware. That pretty much means ACIS ops, @matthewdahmer and occasionally me.

When we make the Ska update request then this can be highlighted if necessary as a notable change.

jzuhone · 2022-02-03T19:20:15Z

@taldcroft the change in the diff I linked should be a no-op, it's just that it's the only part that could conceivably affect model evaluation if I screwed something up.

taldcroft · 2022-02-04T12:18:26Z

@jzuhone - I gave this a spin in ska3-next and it works, but I do see a bunch of new warnings:

The process has forked and you cannot use this CoreFoundation functionality safely. You MUST exec().
Break on __THE_PROCESS_HAS_FORKED_AND_YOU_CANNOT_USE_THIS_COREFOUNDATION_FUNCTIONALITY___YOU_MUST_EXEC__() to debug.

@javierggt will probably have a new 2022.2rc4 release out shortly that includes sot/ska_matplotlib#24, which changes the default plot_cxctime color (discussed previously on slack). Since this PR won't go in until after 2022.2 (sot/skare3#774) you might as well start testing there.

taldcroft · 2022-02-04T12:24:13Z

One feature request (and maybe it's already there?) is to be able to get any flight model with the name, e.g.

xija_gui_fit aca --stop=2022:001 --days=200 --set-data='aca0=-10'

This would grab the latest flight model from chandra_models with xija_get_model_spec for any model_name that doesn't end in .json. Or something like that.

jzuhone · 2022-02-04T23:53:01Z

@taldcroft

when are you getting this warning?

The process has forked and you cannot use this CoreFoundation functionality safely. You MUST exec().
Break on __THE_PROCESS_HAS_FORKED_AND_YOU_CANNOT_USE_THIS_COREFOUNDATION_FUNCTIONALITY___YOU_MUST_EXEC__() to debug.

I don't see it.

… be taken from chandra_models

…s no saved model at all), ask if you want to save

taldcroft · 2022-02-07T12:32:34Z

Huh, I tried again and I don't see that now. Well, never mind for the moment.

matthewdahmer · 2022-02-07T13:01:59Z

@taldcroft @jzuhone I saw that same error, "__THE_PROCESS_HAS_FORKED...", once on my Apple Silicon Mac. During that instance, once I saw it I was not able to get xija_gui_fit to run until I rebooted, then it worked fine.

jzuhone · 2022-02-07T13:28:38Z

Weird. Anyway, I added the new feature @taldcroft requested. It also prompts if you have an unsaved model when you hit the Quit button.

taldcroft · 2022-02-07T14:22:51Z

I added the new feature @taldcroft requested. It also prompts if you have an unsaved model when you hit the Quit button.
Nice! I also like the new Thaw/Freeze boxes, when I was using the previous version I had to think for a few seconds to remember the right words.

One more feature request - put the xija version into the App title? In testing just now I had been accidentally getting the wrong version so it would be nice to see the version.

taldcroft

I did some more light testing of this in ska3-next (rc4) and didn't see any problems, so good from my perspective. Note that I did not look at the code changes.

taldcroft · 2022-02-07T19:32:55Z

xija/gui_fit/app.py

@@ -1082,7 +1082,8 @@ def set_title(self):
            title_str = "no filename"
        if not self.checksum_match:
            title_str += "*"
-        self.window.setWindowTitle(f"xija_gui_fit ({title_str})")
+        self.window.setWindowTitle(
+            f"xija_gui_fit v{xija.__version__} ({title_str})")


jzuhone · 2022-02-10T21:59:52Z

Ok to merge this before things get too busy?

taldcroft · 2022-02-11T10:37:32Z

Fine by me to merge, but @matthewdahmer - are you good with this?

matthewdahmer · 2022-02-11T14:32:37Z

@jzuhone @taldcroft I am OK with this update. I tested it on my Mac with a PLINE refit and seems to work well!

taldcroft · 2022-02-11T16:08:28Z

BTW @jzuhone have you tested this with ska3-next on supported platforms?

I guess that isn't a lien on merging this since we can always come back with fixes as needed. My light testing was OK on that but it's worth noting that @javierggt experienced troubles with aca_view (another Qt app) and required export QT_MAC_WANTS_LAYER=1 to get it to work. (It is slightly perplexing that xija_gui_fit works without that).

jzuhone · 2022-02-11T16:24:25Z

@taldcroft I have not yet tested against ska3-next--is there a documented way yet to set up a ska3-next environment?

taldcroft · 2022-02-11T16:31:40Z

@jzuhone - the conda command under Testing in sot/skare3#774 should work.

jzuhone · 2022-02-14T04:09:21Z

@taldcroft I tested under ska3-next and all appears to work fine.

taldcroft · 2022-02-14T11:29:24Z

👍

jzuhone · 2022-02-14T19:50:58Z

@taldcroft what's your position on who should merge this?

jzuhone added 15 commits December 21, 2021 15:59

Simplify this using np.quantile

3e82926

Remove the console

3a278d9

A way to distinguish between "bad times" and masked out times that on…

183a8cc

…e adds later

Git rid of command entry altogether and move freeze, thaw, ignore, no…

60309fb

…tice to different elements. Fix some bugs.

Remove print statement

56e2039

Fix a remarkably annoying bug that would not allow time plots to scal…

5e48eb5

…e properly

Fix very screwed up ignore/notice logic

305f2ff

Add a splitter

c83db68

Remove unnecessary uses of "object" subclassing

6f53deb

Remove utils file as it is no longer needed

32a41ad

Allow for a new bad time to be added

c2e44b4

Refactor of ignore/notice and bad times

0c41740

Refactor the error box and use it elsewhere

e62df78

Nit

85ab04d

Some pretty hacky stuff going on here, but it works

693e5eb

jzuhone added 2 commits February 5, 2022 16:38

Allow the filename command line argument to be the name of a model to…

d84a249

… be taken from chandra_models

If model in memory is different from the model on disk (or if there i…

c76a819

…s no saved model at all), ask if you want to save

taldcroft approved these changes Feb 7, 2022

View reviewed changes

put version in window title

9cc0dc4

taldcroft reviewed Feb 7, 2022

View reviewed changes

Make button name clearer

ff688aa

taldcroft merged commit d07f9cd into sot:master Feb 14, 2022

javierggt mentioned this pull request Mar 18, 2022

Update sot/xija to 4.25.0 sot/skare3#805

Closed

javierggt mentioned this pull request Mar 30, 2022

2022.3 sot/skare3#812

Merged

javierggt mentioned this pull request May 18, 2022

2022.5 (closed) sot/skare3#850

Closed

javierggt mentioned this pull request Aug 3, 2022

2022.5 sot/skare3#899

Merged

jzuhone deleted the fix_gui_fit branch September 7, 2022 16:38

javierggt mentioned this pull request Jan 4, 2023

Update sot/xija to 4.19.2 sot/skare3#986

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates for xija_gui_fit #116

Updates for xija_gui_fit #116

jzuhone commented Feb 3, 2022

jzuhone commented Feb 3, 2022

taldcroft commented Feb 3, 2022

jzuhone commented Feb 3, 2022

taldcroft commented Feb 4, 2022

taldcroft commented Feb 4, 2022

jzuhone commented Feb 4, 2022

taldcroft commented Feb 7, 2022

matthewdahmer commented Feb 7, 2022

jzuhone commented Feb 7, 2022

taldcroft commented Feb 7, 2022

taldcroft left a comment

taldcroft Feb 7, 2022

jzuhone commented Feb 10, 2022

taldcroft commented Feb 11, 2022

matthewdahmer commented Feb 11, 2022

taldcroft commented Feb 11, 2022

jzuhone commented Feb 11, 2022

taldcroft commented Feb 11, 2022

jzuhone commented Feb 14, 2022

taldcroft commented Feb 14, 2022

jzuhone commented Feb 14, 2022

Updates for xija_gui_fit #116

Updates for xija_gui_fit #116

Conversation

jzuhone commented Feb 3, 2022

Description

Testing

jzuhone commented Feb 3, 2022

taldcroft commented Feb 3, 2022

jzuhone commented Feb 3, 2022

taldcroft commented Feb 4, 2022

taldcroft commented Feb 4, 2022

jzuhone commented Feb 4, 2022

taldcroft commented Feb 7, 2022

matthewdahmer commented Feb 7, 2022

jzuhone commented Feb 7, 2022

taldcroft commented Feb 7, 2022

taldcroft left a comment

Choose a reason for hiding this comment

taldcroft Feb 7, 2022

Choose a reason for hiding this comment

jzuhone commented Feb 10, 2022

taldcroft commented Feb 11, 2022

matthewdahmer commented Feb 11, 2022

taldcroft commented Feb 11, 2022

jzuhone commented Feb 11, 2022

taldcroft commented Feb 11, 2022

jzuhone commented Feb 14, 2022

taldcroft commented Feb 14, 2022

jzuhone commented Feb 14, 2022