Implementing lazy-loading

[edan-bainglass]

This PR aims to improve the user's experience by isolating loading of widgets/resources to where they are needed (requested).

The overall approach is as follows (per widget):

1. Move bulk of __init__ logic to a new render method
2. Add a loading widget (introduced in this PR) as a child in __init__
3. Observe the container of the widget (widget might be in an accordion, a tab, etc. -> observe the selected_index trait of the container)
4. On observable notification (e.g., tab switch), trigger widget's render method

Additionally, I take here an MVC (Model-View-Controller) approach to the app. The app's logic is migrated out to Model classes that represent the so-called "business-logic" of the app. Ideally, though perhaps not quite there yet, it should be possible to prepare and submit a QE calculation entirely via the Model classes!

A combination of a View and a Controller, here represented in a single class, is then simply a "window" to the Model's data. The View (the widgets) represent the UI. The Controller (the _on_<event> methods) is in charge of reacting to events either triggered by the user via the View, or triggered internally via the Model network.

Note that as I worked on this PR, several competing "flavors" of MVC came about. The present implementation is not a settled one. There are still some issues to address in order to properly establish our standard. One major issue is described below.

Open design concern

Currently, the ConfigurationModel, which represents the data of step 2, is injected into all other SettingsPanels. There, traits of the corresponding SettingsModel can be linked to the corresponding traits in the ConfigurationModel. This effectively syncs the specific settings with the app's configuration. There are presently a couple of idea to resolve this coupling between settings. Glad to discuss with those who are interested.

Glad to adjust where necessary 🙂

Update

Recent changes decouple the ConfigurationModel from the SettingsPanels. The AdvancedSettings class is still coupled with its "sub-settings", e.g., smearing, magnetization, etc. This is primarily due to the highly coupled set_model_state and get_model_state methods of the AdvancedModel. But this okay for now, and perhaps in general, as the advanced settings is presented all together in one panel.

The decoupling hinges on a mechanism of announcing dependencies. For example, the PDOS plugin requires both the input structure and the protocol. Hence, it defines

class PdosModel(SettingsModel):
    dependencies = [
        "input_structure",
        "workchain.protocol",
    ]

    input_structure = tl.Instance(orm.StructureData, allow_none=True)
    protocol = tl.Unicode(allow_none=True)
...

During plugin discovery, these dependencies are used to set up directional links, connecting the model to the configuration model network.

Note, however, that in decoupling the ConfigurationModel, I made a decision to extract the structure relaxation and property selection parts out from the basic settings. The new design of the configuration step is now split into sub-steps (thanks @mikibonacci for the suggestion to refer to these as step 2.x).

Glad to discuss and adjust where necessary 🙂

Documentation

Of course, once the PR is approved, documentation of the settled approach (or at least the current one) will be added!

[edan-bainglass]

@PNOGillespie Hi Peter. I'm reaching out regarding this PR in hope that you may assist with one part relating to your XAS plugin.

To keep it brief, the settings module of your XAS plugin triggers a good deal of logic on import. This prevents the implementation of lazy-loading w.r.t XAS.

A simple solution would be to do as @superstar54 did in the XPS plugin, i.e., wrap such logic in a function to be lazily triggered on demand. Do you think this is something you could look into in a separate PR? If not, could you please comment on the reason?

[PNOGillespie]

Hi @edan-bainglass. I can move the logic for updating the pseudopotentials (which is what lines 67 - 112 in settings do on starting the plugin) to be under _update_element_select_panel(). This should mean that the logic is only triggered by _update_structure(). Is that what you meant? Please let me know if I'm missing something.

I'm somewhat busy both this week and next, but I should be able to put together a separate PR soon for the changes if it's just a case of moving things around.

[edan-bainglass]

Hi @PNOGillespie 👋

Hi @edan-bainglass. I can move the logic for updating the pseudopotentials (which is what lines 67 - 112 in settings do on starting the plugin) to be under _update_element_select_panel(). This should mean that the logic is only triggered by _update_structure(). Is that what you meant? Please let me know if I'm missing something.

Yes, this sounds right. As stated, @superstar54's approach in the XPS plugin is likely the way to go, for reference 🙂 Once the PR is ready, I test against my lazy-loading work and we see more clearly if it's sufficient 👍

I'm somewhat busy both this week and next, but I should be able to put together a separate PR soon for the changes if it's just a case of moving things around.

No worries. The lazy-loading work will take a few pass (possibly incremental PRs), so the XAS part can be addressed once your PR is in.

Thanks 🙏

[edan-bainglass]

@PNOGillespie @superstar54 just wondering, why were the XPS and XAS plugins included in the core app? Would it be better to maintain them as external plugins, say aiidalab-qe-xps and aiidalab-qe-xas, to be installed by those interested? What do you think?

[edan-bainglass]

Hi all. The PR is ready for review. As mentioned, it is massive! If necessary, maybe we split the work a bit, say, one app step per person? Not sure if this helps. Anyhow, I'm available for questions :)

[mikibonacci]

I would add some details here, like "Please select a custom ecut and ecutrho"... But actually,
the get_builder_from_protocol will except because indeed no cutoff are found. So,
We can put actuallly a "please select another pseudopotential family". This is kind of a bug
from aiida-pseudo, hopefully we can solve it easily

[edan-bainglass]

Agreed! Could you open an issue (to be PR'd soon)? 🙏

[danielhollas]

@edan-bainglass it would be very helpful to me if you could point me to the things you want me to look at, I am feeling a bit lost right now. :-) Or we can wait till Monday, where you will present the approach in general?

[edan-bainglass]

@edan-bainglass it would be very helpful to me if you could point me to the things you want me to look at, I am feeling a bit lost right now. :-) Or we can wait till Monday, where you will present the approach in general?

Best if I walk you through it on Monday. However, in the meantime, you can take a look at #919, where I've started documenting the core principles.