Template resources setting

[unkcpz]

Supersede #483, fixes #482 also fixes https://github.com/aiidalab/aiidalab-qe/issues/240

• [X] UI re-organized by having resource database select in quick setup and having a switch for only detailed setup
• [X] Refactoring and decouple quick setup
• [x] Decouple detailed setup
• [x] Unit test for resource widgets.
• [x] Help text for detailed setup, when only detailed setup don't use tab widget.
• [x] layout reorganized for quick setup. Having account/password aligned as normal login UI.
• [x] Check XXX 001
• [x] Actual template implementation for computer and code, check #483 for the demo.
• [x] Check all XXX label 002 is cleaned.
• [x] glitches fixes:
  • [x] reset and change computer, the template of code not show up.
  • [x] the help text not disappeared after select new code without template fields.
  • [x] database widget reset should alse reset the option of sub-widgets

Feedbacks from @giovannipizzi after demo in the aiidalab meeting, can add in the template a section "meta" for information like

• [x] which variables can be set for template fields. ("pw", "ph" ... the valid binary name for QE)
• [x] The mapping from the template variable to the name prompted showing in the widget. ("slurm_account" -> "Slurm account").

More tasks after @superstar54's test on CSCS machine.

• [x] Since CSCS needs 2FA now, it makes no sense to ask for password (maybe they didn't deprecate password for all users?). The solution then become using aiidalab-mfa-cscs, and having a template metadata field that can display the information.
• [x] default value read and check with option list
• [x] Suppress entry points not found issue when it is a template
• [x] The template should be valid if the entry point matches with the template.
• [x] The template field can continue to vary, means can reset in place.

More tasks after discuss with @yakutovicha.

• [x] The username now can be template for the computer_configure. I think this is a very good design decision, we were not able to put username which is a mandatory field for verdi computer configure since we don't want the code/resoruce registry contain specific information. Now thanks to the flexible template mechanism this information can be asked without specify the final value in the registry setup file.
• [x] The detail setup has different UI layout from quick_setup but share the same traits, combine into one with a toggle to switch the layout can be a better UI design.

Notes for myself:

• [ ] Before merge, instead of relying on the remote aiida-resource-registry database, mock one as fixture locally but validate it with the remote schema, this give us more flexibility to and robustness to run unit test.
• [x] Put template variables specification under metadata.template_variables?
• [x] Adjust UI so the box of detailed setup is align good with description
• [x] Pass the notebook tests.

[codecov[bot]]

Codecov Report

Attention: 60 lines in your changes are missing coverage. Please review.

Comparison is base (5b303df) 79.92% compared to head (4060723) 82.79%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #511 +/- ## ========================================== + Coverage 79.92% 82.79% +2.87% ========================================== Files 27 27 Lines 3815 4313 +498 ========================================== + Hits 3049 3571 +522 + Misses 766 742 -24 ``` | [Flag](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/511/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | Coverage Δ | | |---|---|---| | [python-3.10](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/511/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | `82.79% <92.05%> (+2.87%)` | :arrow_up: | | [python-3.8](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/511/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | `82.83% <92.05%> (+2.87%)` | :arrow_up: | | [python-3.9](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/511/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | `82.83% <92.05%> (+2.87%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/511?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab) | Coverage Δ | | |---|---|---| | [aiidalab\_widgets\_base/utils/\_\_init\_\_.py](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/511?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#diff-YWlpZGFsYWJfd2lkZ2V0c19iYXNlL3V0aWxzL19faW5pdF9fLnB5) | `75.22% <100.00%> (+12.72%)` | :arrow_up: | | [tests/test\_databases.py](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/511?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#diff-dGVzdHMvdGVzdF9kYXRhYmFzZXMucHk=) | `100.00% <100.00%> (ø)` | | | [tests/test\_computational\_resources.py](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/511?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#diff-dGVzdHMvdGVzdF9jb21wdXRhdGlvbmFsX3Jlc291cmNlcy5weQ==) | `99.70% <99.60%> (-0.30%)` | :arrow_down: | | [aiidalab\_widgets\_base/databases.py](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/511?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#diff-YWlpZGFsYWJfd2lkZ2V0c19iYXNlL2RhdGFiYXNlcy5weQ==) | `93.71% <96.80%> (-0.41%)` | :arrow_down: | | [aiidalab\_widgets\_base/computational\_resources.py](https://app.codecov.io/gh/aiidalab/aiidalab-widgets-base/pull/511?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aiidalab#diff-YWlpZGFsYWJfd2lkZ2V0c19iYXNlL2NvbXB1dGF0aW9uYWxfcmVzb3VyY2VzLnB5) | `79.87% <84.94%> (+10.02%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[danielhollas]

@unkcpz I would recommend to merge with master branch, I made some fixes in #502 and #508

[unkcpz]

Thanks @danielhollas, I am thinking about it as well. I rebase it.

[unkcpz]

Hi @superstar54, @yakutovicha, this is ready for review, let me know how you want me to rebase it so you can review it more easily. It is currently every task has its own commit. I can rebase it to:

• Refactoring the Computational resource widget by moving quick_setup/detailed_setup widgets out with adding unit tests.
• New computational resource database widget with adding uint tests.
• Implement the template widget and integrate into quicksetup with unit tests.
• moving the widgets into modules as I did for QeApp in https://github.com/aiidalab/aiidalab-qe/pull/439 (Some module files , especially computational_resources.py quite large, I think it is better to move some widgets to files, this is not included in this PR, not sure if we want to have this or not.)

[superstar54]

Hi @unkcpz thanks for the work. the UI looks more organized than before. I did a test using Quick Setup, but got an error,

When I checked the code, I found there was a projwfc-7.2@daint-mc code, which I did select to setup.

[superstar54]

In the Quick Setup UI, it sets up both the computer and code, but the Label only shows daint-mc, which may make the user a little confused.

[unkcpz]

In the Quick Setup UI, it sets up both the computer and code, but the Label only shows daint-mc, which may make the user a little confused.

Good point! This is read from the template, it is the people who upload the template can set the name for this see https://aiidateam.github.io/aiida-resource-registry/database.json, I change it to "Computer Label"

[unkcpz]

Hi @superstar54, could you try again? I find a bug in my code and it is now fixed see https://github.com/aiidalab/aiidalab-widgets-base/pull/511/commits/f75c79de4a3289e2d0b5e4303790d4918955bf8b

[superstar54]

Hi @superstar54, could you try again? I find a bug in my code and it is now fixed see f75c79d

The error still exists. And there is a new error now.

Request for URL https://unkcpz.github.io/aiida-resource-registry/database.json failed; using cached response
Traceback (most recent call last):
  File "/opt/conda/lib/python3.9/site-packages/requests_cache/session.py", line 231, in _resend_and_ignore
    response.raise_for_status()
  File "/opt/conda/lib/python3.9/site-packages/requests/models.py", line 1021, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 404 Client Error: Not Found for url: https://unkcpz.github.io/aiida-resource-registry/database.json

[unkcpz]

For the new error, it is because I move the aiida-resource-registry, and the update in the pr is in the latest commit. But I have no idea how to fix the previous error. Let's meet in person and debug together when you are in the office.

[unkcpz]

Thanks a lot for the review! Take you time and since it is quite big pr it deserve time to check details carefully.

but the obvious question I have here is: are you sure you want to have separate widgets for quick setup and detailed setup? I see a lot of duplication there.

This has to be I'd say. The logic of these two widgets is quite different. The quick_setup widget uses the computer resource database and template widgets and the set of inner computer/code widgets happens in one step and quick_setup button does the last trigger to set the computer and code to the database. But the detali_setup widget has its widgets directly from computer/code widgets.

I started with didn't separate them and found it was insanely complex to implement the template mechanism because everything was tangled. As you can see from the first couple of commits I did the refactoring and decoupled these two things from ComputatianalResourceWidget, I think after refactoring things became clearer and I can focus on certain parts without worrying about the entanglement of traits huge widget class.

These two things have different orientation, they can appear independently (although I made the detail setup widget shows together with quick_setup widget and put it inside the ComputationalResourceWidget, it is not necessary, the quick_setup can be used directly to set the computer/code). From test point of view, these two widgets serve two different use cases and have different flows of usage.

I see a lot of duplication there.

The widgets and their layouts are quite different and the callback functions have different purposes as well. For the quick_setup widget, the callback function is when new computer/code selected, which is triggered from the database widgets. While for the detail_setup widget, the callback function is for when the "setup" button is clicked. From I understanding, the detail widget is a wrapper of AiidaComputer and AiidaCode widget, while quick_setup widget in my design I make it a composite widget to communicate between ComputerResourceDatabaseWidget and AiidaComputer/AiidaCode widgets.

[yakutovicha]

For the record. We had a meeting with @unkcpz to discuss some design aspects. And we agreed on them. At this point, I am waiting for the review request.

[unkcpz]

Hi @yakutovicha, it is ready to review. I left some notes for myself to clean up, please feel free to add more, the notebook test I didn't fix it because it quite tricky and rely on UI, so I want to leave it after all the design is settled before it is ready to merge.

[unkcpz]

Hi @yakutovicha, these two behaviors are both expected. See my explanation below. I don't know if there are better solutions.

When I display step-by-step widgets, changing the slurm partition does not affect the prepend text. However, when I change the account number, things get updated.

This is because the for the prepend text it contains both slurm partition and slurm account, I monitoring the template string and only start render the template to full string when all the template variables are ready. I was trying to immediately update the template string but it cause the problem that when I change the template of changed value to other value, the template not get update anymore. For example, I fill the slurm_acount with jyu and update the template string "xxxx {slurm_acount} yyyy {slurm_partition} " to "xxxx jyu yyyy {slurm_partiotion}", then the template don't have {{ slurm_acount }} anymore and I can not change the slurm_account to other value. This would lead to problem that user wrongly input the username and correct it later. Therefore design I proposed is to make sure when only use the quick_setup, the behaviors are expected. When the toggle of detailed setup is ticked, it checks the template fields and raises warnings to ask for missing variable.

Also, the computer name template is not updated till I modify the default value. Please correct this.

It is also expected because 1. if the value of widget not changed, it will not trigger the callbacks thus nothing changed. 2. the callbacks are guaranteed to be triggered once you click the detailed_setup checkbox. I guess you encounter this because you first display the detailed setup and then go back to the quick setup to change things. I would regard this is an advance usage and only experts may encounter such scenario.

[unkcpz]

Hi @yakutovicha, there is a final decision I am not so sure about. In the https://aiidateam.github.io/aiida-resource-registry/database.json, the metadata section has a relatively flat nest with tooltip and template variables mixed in the same level with each other.

metadata:
  tooltip: "some tooltip"
  slurm_account: 
    default: none
    type: text
    description: this is a field read from template string

which can be:

metadata:
  tooltip: "some tooltip"
  template_variables:
    slurm_account: 
      default: none
      type: text
      description: this is a field read from template string
     slurm_partition:
      default: debug
      type: list

which one do you think is better? Please also check the schema (https://aiidateam.github.io/aiida-resource-registry/resource.schema.json) there, once we make the release, it will become the API_v1 and not going to change in the near future.

[unkcpz]

Rebased and ready for review @yakutovicha

[unkcpz]

As discussed with @yakutovicha on Zoom, there are the following things need to be polished:

• [x] computer label not updated with the default value
• [x] The slurm_partition and slurm_account in the prepend_text now in principle should be able to be updated immediately after the template with default value is loaded.
• [x] Try to use dlink instead of observe for _on_template_variables_* callbacks.

[unkcpz]

Hi @yakutovicha, I made all the changes as we discussed. You are right that for the weird behavior where the template placeholder not update, the codes are already there. Problem was when the default value does not exist for a template string, the _fill_tempalte function raise the exception and exits the for loop and following computer_setup are skipped.

For using the dlink, it is feasible for the code_setup and the test passed directly. But for the computer_setup_and_configure, I can not make such a change because for the template widget the trait filled_template is computer_setup and computer_configure, while for the resource widget the trait is computer_setup_and_configure. The transform is not working here because what is needed here is to dlink from computer_setup -> self.computer_setup_and_configure["setup"].

[yakutovicha]

As discussed with @unkcpz separately, I will do the remaining changes.

[unkcpz]

Hi @yakutovicha, I fix all the tests and try my best to clear the flow of the template class. Please have a "final" (:crossed_fingers:) look.

[danielhollas]

Congrats @unkcpz, this is a cool feature! 👏