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

fix: nosubmit runs with default cluster config #34

Merged
merged 9 commits into from
Nov 3, 2024

Conversation

RobertArbon
Copy link
Contributor

@RobertArbon RobertArbon commented Nov 2, 2024

This addresses the following issues #30 and #32 and possibly impacts #28.

there were two problems here:

  1. the flag nosubmit was different between the GMX and non-GMX config parsers. This was a simple fix and addresses cli-abfe -nosubmit #32
  2. when using nosubmit the cluster configuration dictionary was set to None which raised an error in the Scheduler.generate_scheduler_file method.

To fix (2) I've taken a guess at what you've intended and what is sensible and made the smallest change I could. Now the nosubmit flag has no influence on the generation of a cluster configuration.

So now nosubmit will generate cluster config files taking into account the number_of_parallel_ligand_jobs and nogpu flags.

I've also made some edits to the gitignore file for the outputs from the examples

@RobertArbon RobertArbon marked this pull request as ready for review November 2, 2024 17:14
Copy link
Collaborator

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

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

Thanks, however could you revert all the black formatting please? it makes it impossible for us to review the code changes

@RobertArbon
Copy link
Contributor Author

RobertArbon commented Nov 2, 2024

Thanks, however could you revert all the black formatting please? it makes it impossible for us to review the code changes

Done - it took a few commits but I got there.

I deleted the other PR - not sure it was appropriate in the first place but now it doesn't make sense.

@philbiggin philbiggin merged commit d96444e into bigginlab:main Nov 3, 2024
2 checks passed
@philbiggin
Copy link
Collaborator

merge this into the main branch. thanks @RobertArbon !

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.

3 participants