Fix code loading bug - #1519
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1519 +/- ##
==========================================
+ Coverage 75.24% 75.28% +0.03%
==========================================
Files 115 115
Lines 7381 7371 -10
==========================================
- Hits 5554 5549 -5
+ Misses 1827 1822 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| """Toggle code selectors conditional on plugin activity and required parameters. | ||
|
|
||
| For a given code (e.g., pw), if at least one active plugin requires it (if any condition | ||
| registered for the code is met), we activate it. | ||
| """ |
There was a problem hiding this comment.
Driveby change - just adding some documentation to clarify what this method is doing. Unrelated to this PR.
| super().confirm() | ||
| if not self.has_process: | ||
| self._submit() | ||
| self.lock() |
There was a problem hiding this comment.
This is the best logical point for the locking mechanism. If step 3 is confirmed, it should be locked to prevent further submissions.
| def refresh_codes(self, filter_codes_for_user: bool = True): | ||
| for _, resource_model in self.get_models(): | ||
| resource_model.refresh_codes(filter_codes_for_user=filter_codes_for_user) |
There was a problem hiding this comment.
Helper method used when loading a process.
| def _link_model(self, model: ResourceSettingsModel): | ||
| for dependency in model.dependencies: | ||
| dependency_parts = dependency.split(".") | ||
| if len(dependency_parts) == 1: # from parent, e.g. input_structure | ||
| target_model = self | ||
| trait = dependency | ||
| else: # from sibling, e.g. workchain.protocol | ||
| sibling, trait = dependency_parts | ||
| target_model = self.get_model(sibling) | ||
| ipw.dlink( | ||
| (target_model, trait), | ||
| (model, trait), | ||
| ) |
There was a problem hiding this comment.
This is a remnant that avoided removal in previous PRs. Effectively, it was leading to a broken state, as the parent _link_model, in additional to already covering this logic, has additional logic that is missed by having this override here (e.g., wiring the locked and blockers traits).
| if process_uuid is not None: | ||
| submission_model.refresh_codes(filter_codes_for_user=False) |
There was a problem hiding this comment.
Th key change - if there's a process, don't filter out codes!
The installation of Quantum ESPRESSO is a background process that triggers if no local QE installation is detected. However, the user should be allowed to submit calculations on remote machines that host some QE installation.
5124180 to
41a2c02
Compare
| if self.installing_qe: | ||
| yield "Installing Quantum ESPRESSO codes..." | ||
|
|
||
| if not self.qe_installed: | ||
| yield "Quantum ESPRESSO is not yet installed" | ||
|
|
There was a problem hiding this comment.
Users should be allowed to proceed with remote submission on a machine that hosts some QE installation.
| self.warning = self._WARNING_TEMPLATE.format( | ||
| warning=f"Code '{default_code}' not found" | ||
| ) | ||
| selected = self.first_option |
There was a problem hiding this comment.
None would block the step, but there may be valid code options, e.g., remote ones.
yakutovicha
left a comment
yakutovicha
left a comment
There was a problem hiding this comment.
The code looks good. I've also tested it for some discovered failures - it works flawlessly now.
2 participants
To prevent users from submitting calculations with unconfigured codes, we filter them out:
aiidalab-qe/src/aiidalab_qe/common/code/model.py
Lines 136 to 142 in 9ba5c42
However, this SHOULD NOT happen when loading a process, since the user cannot submit in that state. But it was, so step 3 was emitting blockers due to unselected codes. Prior to #1513, the blockers didn't do their job of blocking the step, but now they do. So if someone loads the app from a process that was run by someone else (e.g., a downloaded example), step 3 is blocked and step 4 never loads.
This PR adds a
filter_codes_for_userflag injected through the resources refresh mechanism that is generallyTruebut set toFalseif we're loading a process. In addition, the PR relaxes default code handling, falling back on the first available code option if the default code does not exist. Finally, the PR removes the blocking of step 3 w.r.t the local QE installation status, since users should still be allowed to proceed with remote submission.