Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Testing string interpolation for multi cli args #83

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

agoscinski
Copy link
Collaborator

So far the tests only test the support of one argument in the cli args. By using analysis_foo_bar which will be splitted into multiple arguments due to the lags, we test now such this use case.

Initialization of all files required to run workflows. The workflow config files
are updated to work properly. The $PWD environment is removed because the
config_rootdir is now used.

To allow a graph like initialization of workgraph tasks first creating the task
nodes and then linking the additional information including inpouts outputs and
arguments we implemented a temporary hack to overcome internal validations.
To include several bugfixes we upgrade to the version v0.4.10. This is
refuired to specify computer in the metadata of the workgraph task as
this is not possible with the currently pinned version. The workgraph
API has changed therefore we adapt to new API the functions which makes
part of the function calls cleaner. The workaround is still needed but
has been better documented.
The user can specify a computer for a task by the aiida label. A computer be
initialized through the verdi CLI or programatically. This is for now the only
feasible solution to support the whole scope of connections ssh and firecrest
without implementing our own CLI resolution logic.
This allows to specify data items that are only remotely available or
should not be copied over to the aiida internal folder. I did not add
the computer information to the IR because the IR focuses on
representing the unrolling/expansion from the config to a graph and the
computer information is completely independent from this logic.
So far the tests only test the support of one argument in the cli args. By using
`analysis_foo_bar` which will be splitted into multiple arguments due to the
lags, we test now such this use case.
Base automatically changed from remote-data to main January 9, 2025 17:54
@agoscinski
Copy link
Collaborator Author

I see the test pass because it is correctly linked in workgraph, but not in the arguments. To make the tests fail an argument check needs to be done. If one checks for the process merge.py the _aiidasubmit.sh then one sees the difference before

'/Users/alexgo/code/Sirocco/tests/cases/parameters/config/scripts/merge.py' 'analysis'  > 'stdout' 2> 'stderr'

and after the fix

'/Users/alexgo/code/Sirocco/tests/cases/parameters/config/scripts/merge.py' 'analysis' 'analysis'  > 'stdout' 2> 'stderr'

In the second case one can see that the two inputs for the lags have been added to the arguments. This reveals actually a problem with aiida-shell that I was not aware, that the names are not named uniquely. Have to think about this.

Before it only was overwriting the input in the `name_to_input_map`
since multiple inputs can be linked to the same name (with different
coordinates)

TODO test that detects this error
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant