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

High throughput wc #147

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

Conversation

federicazanca
Copy link
Collaborator

Still very messy, just making the PR so we can actually see the code when discussing.

Ignore the comments in the model file, will remove and use them to fix other issues.

I made a WorkChain for submitting high-throughput screening, but I am facing the problems described in #146

I also added the submission script for submitting the actual Workchain (in examples/workflows)

And a script to run high-throughput screening without writing a WorkChain. We might end up only keeping this? Depending on discussion on how to implement some things

@federicazanca federicazanca added enhancement New feature or request aiida-mlip to help filter the issues labels Jul 5, 2024
@federicazanca federicazanca self-assigned this Jul 5, 2024
pyproject.toml Outdated Show resolved Hide resolved
examples/workflows/html/hts_workflow.html Outdated Show resolved Hide resolved
examples/workflows/run_hts_ Outdated Show resolved Hide resolved
aiida_mlip/workflows/hts_workgraph.py Outdated Show resolved Hide resolved
@federicazanca
Copy link
Collaborator Author

federicazanca commented Jul 31, 2024

UPDATES

The high throughput WorkChain is now a WorkGraph instead. The WorkChain is cancelled.
I also put scripts in the example to submit it and also to submit many calculations without the need of the workgraph or workchain. Both options are valid and depend on the need of the user.
The tests fail (at least in my computer) probably because of some problem in submitting to a queue. --> discuss with aiida people/fix after cloud is set up. At the moment I made the test just to check that the process is created, but this should be changed

@federicazanca federicazanca marked this pull request as ready for review July 31, 2024 15:04
return wg


def HTSWorkGraph(folder_path: Path, inputs: dict) -> WorkGraph:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should be create_hts_workgraph or something else more in the style of a function name?

Also, is this specific to running geomopt, or can it be generalised to other calculations? Assuming it's specific, which I think it is, it's quite confusing.

wg = WorkGraph("hts_workflow")

wg.add_task(
run_opt_calc, name="opt_task", folder=folder_path, janus_opt_inputs=inputs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be generalised to take an arbitrary task using one of the function's parameters, so it wouldn't have to be hardcoded?

Comment on lines +10 to +14
folder_path = Path("/home/federica/aiida-mlip/tests/workflows/structures/")
inputs = {
"model": ModelData.from_local(
"/home/federica/aiida-mlip/tests/data/input_files/mace/mace_mp_small.model",
architecture="mace_mp",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Paths should be generalised

@@ -30,7 +30,8 @@ python = "^3.9"
aiida-core = "^2.6"
ase = "^3.23.0"
voluptuous = "^0.14"
janus-core = "^v0.6.0b0"
janus-core = "^v0.6.3b0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might need to pin or check compatibility with newer versions

Comment on lines +20 to +25
# AT THE MOMENT WE ONLY CHECK THE PROCESS IS CREATED AT LEAST,
# WHEN WE FIX THE SUBMISSION THIS NEEDS TO BE CHANGED

assert wg.state == "CREATED"
# wg_node = load_node(wg.pk)
# assert isinstance(wg_node.outputs.opt_structures.h2o, StructureData)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs updating

"code": janus_code,
}
wg = HTSWorkGraph(folder_path=structure_folder2, inputs=inputs)
wg.wait(15)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there nicer ways of doing this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aiida-mlip to help filter the issues enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants