Bump SSSP to 1.3

[unkcpz]

• The test fail because the default value will read from aiida-quantumespresso where the pseudo version in the protocol not updated yet.

[unkcpz]

As discussed in AiiDA meeting, I move on without the protocol change in aiida-quantumespresso. But it means we need to explicitly override the pseudo_family when creating a builder.
@superstar54 When I make these changes, I see two things we can improve after we back from Christmas break, 1) the unit tests and fixtures must be refactored, there are too many duplicates and quite a mess. 2) In aiida-quantumespresso, the ProtocolMixin is used and there are protocols defined by yaml files for every workchains. We probably use the same way rather than override it in the code, however, I see this against the plugin where the get_builder is used and the builder is created out of air.

Anyhow, I don't want to block the this SSSP bump which Nicola asks for the QeApp explicitly.

[unkcpz]

There still couple of tests failed and I have no idea how to fix those, I guess it requires to haveing a proper parameters passed to every workchain creator fixture. @superstar54 Can you have a look?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (83bfea6) 80.74% compared to head (a0f03cd) 80.86%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #578      +/-   ##
==========================================
+ Coverage   80.74%   80.86%   +0.12%     
==========================================
  Files          49       49              
  Lines        3418     3429      +11     
==========================================
+ Hits         2760     2773      +13     
+ Misses        658      656       -2     

Flag	Coverage Δ
python-3.10	80.86% <100.00%> (+0.12%)	⬆️
python-3.8	80.91% <100.00%> (+0.12%)	⬆️
python-3.9	80.91% <100.00%> (+0.12%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[superstar54]

There still couple of tests failed and I have no idea how to fix those, I guess it requires to haveing a proper parameters passed to every workchain creator fixture. @superstar54 Can you have a look?

Hi @unkcpz which one? I saw all the tests passed.

[superstar54]

1. the unit tests and fixtures must be refactored, there are too many duplicates and quite a mess. 2) In aiida-quantumespresso, the ProtocolMixin is used and there are protocols defined by yaml files for every workchains. We probably use the same way rather than override it in the code, however, I see this against the plugin where the get_builder is used and the builder is created out of air.

1. Indeed, we need to update the tests and fixtures. Initially, when I wrote the fixture for the workchain and plugin-related tests, the concept of the plugin was still in its early stages. Now, with the plugin architecture having stabilized, we could update the test with a better mind.

2. Using ProtocolMixin, one defines all the parameters in the YAML file. When creating the builder, it fetches these parameters based on the protocol . However, QEapp has a different way. In QEapp, one can still define the protocol and related parameters in the YAML file. But when the user selects the protocol from the GUI, the GUI will fetch the corresponding parameter immediately and show the values to the user. At this stage, the user can modify the GUI to override the parameters. In the final submission step, we read all the parameters from the GUI (the protocol value and the user override values) and use these values to create the builder, so we don't need to read the YAML file in the get_builder function again.
   Unless we want to define some other protocol-related parameters that don't have their corresponding value in the GUI, we can read them in the get_builder function. By the way, the get_builder function is quite flexible. The plugin developer can define the YAML and load it into the get_builder. If this is a common use case, one can improve the get_builder function to standardize it.

   @mbercx knows better ProtocolMixin, feel free to comment.

[unkcpz]

Hi @unkcpz which one? I saw all the tests passed.

Ha, nice. Some failed in my local environment, I guess I messed up with the environment somehow. Can you give this branch a test?

I also suggest testing opening a finished workchain after updating the app. In this PR, I also fix an issue that the post_install of pseudo library will fail in the AppStore.

[unkcpz]

Thanks for the very detailed comment, I’ll convert it to an issue for future improvements. For the PR itself, can you give it a look? I want to make another release and build the quantum mobile again on Monday. Sent from Outlook for iOS<https://aka.ms/o0ukef>

…

________________________________ From: Xing Wang ***@***.***> Sent: Thursday, December 14, 2023 11:20:47 PM To: aiidalab/aiidalab-qe ***@***.***> Cc: Jusong Yu ***@***.***>; Mention ***@***.***> Subject: Re: [aiidalab/aiidalab-qe] Bump SSSP to 1.3 (PR #578) 1. the unit tests and fixtures must be refactored, there are too many duplicates and quite a mess. 2) In aiida-quantumespresso, the ProtocolMixin is used and there are protocols defined by yaml files for every workchains. We probably use the same way rather than override it in the code, however, I see this against the plugin where the get_builder is used and the builder is created out of air. 1. Indeed, we need to update the tests and fixtures. Initially, when I wrote the fixture for the workchain and plugin-related tests, the concept of the plugin was still in its early stages. Now, with the plugin architecture having stabilized, we could update the test with a better mind. 2. Using ProtocolMixin, one defines all the parameters in the YAML file. When creating the builder, it fetches these parameters based on the protocol . However, QEapp has a different way. In QEapp, one can still define the protocol and related parameters in the YAML file. But when the user selects the protocol from the GUI, the GUI will fetch the corresponding parameter immediately and show the values to the user. At this stage, the user can modify the GUI to override the parameters. In the final submission step, we read all the parameters from the GUI (the protocol value and the user override values) and use these values to create the builder, so we don't need to read the YAML file in the get_builder function again. Unless we want to define some other protocol-related parameters, that don't have their corresponding value in the GUI, we can read the YAML in the get_builder function. By the way, the get_builder function is quite flexible. The plugin developer can define the YAML and load it into the get_builder. If this is a common use case, one can improve the get_builder` function to standardize it. @mbercx<https://github.com/mbercx> knows better ProtocolMixin, feel free to comment. — Reply to this email directly, view it on GitHub<#578 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACDMFRMM4BMCSS4G2ONXRULYJN3T7AVCNFSM6AAAAABARYVQXKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJWG44DSNRZGQ>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[unkcpz]

I merged this anyway so as not to block the release of Quantum Mobile, in principle I should not merge it without approve.

[unkcpz]

Following tests are all good:

• Install the app in a full-stack image, the pseudo library and qe are all installed.
• Start from the 23.10.0rc0 where the SSSP version is v1.2, update the app from appstore the pseudo libraries of SSSP 1.3 installed automatically and the app shows the progress bar to block the submission step until the pseudo families installed correctly.
• Start from a bare qe image the SSSP 1.3 is installed and ready to launch a calculation.