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

Base RaspaBaseWorkChain on BaseRestartWorkChain from aiida-core. #56

Merged
merged 3 commits into from
Jul 7, 2020

Conversation

yakutovicha
Copy link
Collaborator

No description provided.

@yakutovicha
Copy link
Collaborator Author

@pzarabadip I noticed a few things here:

  • example_base_workchain_gcmc_2comp.py did not converge at the end.

  • examples/workchains/example_base_workchain_widom_1comp.py did not finish after 5 iterations.

  • examples/workchains/example_base_workchain_gemc_1comp.py does not call handlers.

Can you have a look, please? Or should we just merge this PR and make a separate issue for those problems?

@yakutovicha
Copy link
Collaborator Author

this can only be merged after aiidateam/aiida-core#3786.

@ezpzbz
Copy link
Collaborator

ezpzbz commented Feb 29, 2020

Sure @yakutovicha. I will check them and will let you know.

@yakutovicha yakutovicha force-pushed the base_restart_from_aiida branch 2 times, most recently from c99f856 to 68d3532 Compare March 18, 2020 19:23
@ezpzbz
Copy link
Collaborator

ezpzbz commented Mar 30, 2020

@pzarabadip I noticed a few things here:

  • example_base_workchain_gcmc_2comp.py did not converge at the end.
  • examples/workchains/example_base_workchain_widom_1comp.py did not finish after 5 iterations.
  • examples/workchains/example_base_workchain_gemc_1comp.py does not call handlers.

Can you have a look, please? Or should we just merge this PR and make a separate issue for those problems?

I just checked these examples @yakutovicha
The reason that these are not being converged is that additional_init_cycle and additional_prod_cycle are hard-coded to 0 in handlers.
It would be great if we can make them as well as conv_threshold user-defined. So, user can pass them to builder for instance like:

builder.handler_overrides = Dict(dict={
        'check_gcmc_convergence': True,
        'conv_threshold': 0.2,
        'additional_init_cycle': 1000,
        'additional_prod_cycle': 2000
    })

I will mark the parts that needs to change separately.

Copy link
Collaborator

@ezpzbz ezpzbz left a comment

Choose a reason for hiding this comment

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

This review contains changes to fully enable checking the simulations from the convergence point of view and benefiting from handlers to solve the convergence issues if they occur.

aiida_raspa/workchains/base.py Outdated Show resolved Hide resolved
aiida_raspa/workchains/base.py Outdated Show resolved Hide resolved
aiida_raspa/workchains/base.py Outdated Show resolved Hide resolved
@yakutovicha
Copy link
Collaborator Author

It would be great if we can make them as well as conv_threshold user-defined. So, user can pass them to builder for instance like:

Unfortunately, handlers do not accept additional input parameters..

* Migrate all the examples to the new API
* Remove redundant files (old base restart and related things)
* Inject all process_handlers in the base restart work chain.
@yakutovicha yakutovicha force-pushed the base_restart_from_aiida branch 2 times, most recently from 712648f to 7c582e7 Compare July 7, 2020 18:39
@yakutovicha yakutovicha merged commit aa3d40b into develop Jul 7, 2020
@yakutovicha yakutovicha deleted the base_restart_from_aiida branch July 7, 2020 19:27
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.

2 participants