Image is generated twice when loading

[pshriwise]

When the application is first opened, the functions for the id/material mapping are called twice.

[kkiesling]

I am not quite sure I can reproduce this. When you open the application, do you see Reading materials XML file... appear twice in the terminal? This is my terminal output when I load the test geometry in this repo. I don't see indication of double loading here.

[pshriwise]

OpenMC isn't initialized twice, but a redundant call to openmc.lib.id_map and/or openmc.lib.propety_map occurs.

[kkiesling]

I think I figured out how it is getting called twice (or even more). Bare with me, this is a bit long, but I will link to as much source as possible.

So first, we know that makePlot() is being called 2x upon loading, when it should only be called once. Through backtrace, this is getting called through both loadGui() and applyChanges() (both call generatePixMap() --> generatePlot() --> makePlot()). Presumably, upon start up this should only be called with loadGui as there should be no "changes" to apply.

However, applyChanges() can also be called through updating menu options. In this case it is also being called on startup by various toggled options through the edit menu. The edit menu is updated here (in loadGui at startup) by calling updateEditMenu(). These three edit options are what is ultimately calling applyChanges() here, here, and here.

Now here is the complicated thing, those three options (masking, highlight, outline) have their state saved from the previous loading of the GUI. They are present in the Edit menu ("Enable ...") - shown in pic A.

Pic A:

If any of those three options are checked (but not the "Enable Overlap Coloring" option, not sure why that one is excluded), the check is saved for the next load. Pic B shows menu with all options checked before exiting. Pic C shows which options were checked after reloading, which means applyChanges() is called up to 3 times (once per option checked). If any of the 3 options in question are left unchecked, then they remain unchecked and applyChanges() is skipped.

Pic B: ]

Pic C:

---

So the TL;DR is that the state of the edit menu is saved for those three edit menu options and if they are checked, the call to makePlot() is essentially called for each checked item (so these methods in question can be called up to 3 extra times upon start up right now).

[kkiesling]

You can reproduce the behavior by playing around with toggling those options before quitting the gui. If you uncheck all of them, you should have no additional makePlot calls than just the single one for loading the GUI.

[pshriwise]

Thanks for digging into that @kkiesling!

Tangentially, I didn't know the menu items would show up so nicely on OSX. That's cool to see 😁

[kkiesling]

So I guess the question on moving forward is, do you want the behavior of the GUI to be such that the state of the edit menu is saved? Because if not, I think it is just a simple removal of these 3 lines to stop the additional loading at start up. But if you want the state to be saved, then I think there's not a way around the additional loading.

[pshriwise]

I do think it would be good for those settings to persist. When users are post-processing data it's inconvenient to have to re-select settings each time they open the plotter. Ideally, we can postpone the connections for state changes of these options to the data model until after the information from the plot_settings.pkl file has been applied.

[pshriwise]

I haven't looked into how much restructuring that would require, but I'm guessing it may require that we have separate methods to connect the different GUI objects to their respective methods. We can call those after the state of the objects has been set. IIRC those connections are made in the constructors right now.

[kkiesling]

I am definitely a novice with QT and GUI based things.. BUT it seems like it is not actually possible to prevent reloading if you want those checks to stay on in the edit menu. After the first (and desired) call to makePlot() all other calls are executed during app execution at this line. And each time those options are toggled either on or off (from the gui while being used), makePlot() is triggered. So I think any place you want to set the state of those toggles, even through the loading of the plot_settings.pkl file, it will trigger the loading during execution anyway.

[pshriwise]

I'm fairly certain that a lot of the calls to openmc.lib.id_map are occurring in the call to main.loadGui beforehand. This is where the model state is read in from the plot_settings.pkl file. If you place print statements in PlotImage.generatePixmap and right above the line you indicated you'll see that several calls to PlotImage.generatePixmap (and in turn to openmc.lib.id_map) occur before we get to the sys.exit(app.exec()) call. These are triggered by callbacks set on the menu items to update the image data when changed in main.loadGui

We need a way to disable calls to openmc.lib.id_map until we're done loading model information and setting its state. I think some kind of attribute on the PlotImage object like "frozen" or something that's set to True during loadGui and False at the end would do the trick.

[pshriwise]

Since I had it all in my head, I went ahead and took a crack at it in this branch. What do you think?

[kkiesling]

That's definitely an easier solution than I anticipated it being. It does the trick for me when I just tried it.

This does remind me of another question- do you want the "Enable Overlap Coloring" setting to be saved like the other three options? Right now it is not setup to be saved.

[pshriwise]

Awesome! I'll go ahead and submit a PR since I have it coded up already.

Yeah, ideally that setting would persist too. Want to tackle that one?

[kkiesling]

Yep, can do

[paulromano]

Closed with the merging of #90