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

Add utility for matrix builds #373

Merged
merged 4 commits into from
Apr 29, 2022
Merged

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Apr 22, 2022

Often, developers want to test a given set of test cases or a test suite with multiple compilers and configurations. Doing this process manually can be challenging and prone to mistakes. The setup_matrix.py scrip is designed to automate this process with the help of a config file similar to example.cfg. See the README.md added as part of this PR.

@xylar xylar added enhancement New feature or request utility Utility script(s) outside of the compass package labels Apr 22, 2022
@xylar xylar requested a review from sbrus89 April 22, 2022 18:08
@xylar xylar self-assigned this Apr 22, 2022
@xylar xylar marked this pull request as draft April 22, 2022 18:12
@xylar xylar force-pushed the add_matrix_build branch 2 times, most recently from dc69b6b to 2af2b31 Compare April 24, 2022 11:53
@xylar
Copy link
Collaborator Author

xylar commented Apr 24, 2022

I've tested this some but it seems prohibitively slow to do each build one after the other. Building each configuration in parallel with each other, on the other hand, is going to be tricky, requiring a different conda environment and worktree for each.

Also, it seems like we need to generate a job script for each setup. Otherwise, it's too hard to run everything in interactive jobs. I think I might want to introduce that capability before the build matrix.

@xylar xylar force-pushed the add_matrix_build branch from 2af2b31 to 5a2bb1d Compare April 27, 2022 16:03
@pep8speaks
Copy link

pep8speaks commented Apr 27, 2022

Hello @xylar! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 1:22: W292 no newline at end of file

Comment last updated at 2022-04-29 07:06:32 UTC

@xylar xylar force-pushed the add_matrix_build branch 2 times, most recently from 93ef253 to 6aa0443 Compare April 27, 2022 16:53
@xylar xylar marked this pull request as ready for review April 27, 2022 20:01
@xylar
Copy link
Collaborator Author

xylar commented Apr 27, 2022

@sbrus89, I will continue to test this but it seems to be working. I am going to use it to test a different branch where I will be adding PETSC and LAPACK. If you have time to review and give feedback, this PR is the highest priority for me among the ones where I have asked for your review.

I know you're still recovering so no pressure, just wanted to let you know which is a priority for me.

@xylar xylar force-pushed the add_matrix_build branch from 6aa0443 to 49dc790 Compare April 27, 2022 20:06
@sbrus89
Copy link
Collaborator

sbrus89 commented Apr 28, 2022

@xylar, great; thanks for letting me know. I'm going through this and testing it out now.

Copy link
Collaborator

@sbrus89 sbrus89 left a comment

Choose a reason for hiding this comment

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

@xylar, this worked wonderfully for me. It's going to be particularly awesome when combined with #376.

The only other thing I'll mention is that it would be nice to have a --submit to kick off each job in the matrix, once this has been merged and the job script capability is available.

@sbrus89 sbrus89 added enhancement New feature or request and removed enhancement New feature or request labels Apr 28, 2022
@sbrus89
Copy link
Collaborator

sbrus89 commented Apr 28, 2022

Also, just thinking out loud here: could the build for each matrix member be included in the job script?

@xylar
Copy link
Collaborator Author

xylar commented Apr 28, 2022

Also, just thinking out loud here: could the build for each matrix member be included in the job script?

It could be but if you submitted all the jobs at once, they would mess each other up. The build can't happen in parallel in the same directory.

@sbrus89
Copy link
Collaborator

sbrus89 commented Apr 28, 2022

Also, just thinking out loud here: could the build for each matrix member be included in the job script?

It could be but if you submitted all the jobs at once, they would mess each other up. The build can't happen in parallel in the same directory.

very true.

@xylar
Copy link
Collaborator Author

xylar commented Apr 28, 2022

The only other thing I'll mention is that it would be nice to have a --submit to kick off each job in the matrix, once this has been merged and the job script capability is available.

I like that idea very much. I don't see any downside since you should be able to override the default wall-clock time, account, etc. if the deafults don't make sense.

@xylar
Copy link
Collaborator Author

xylar commented Apr 28, 2022

It could be but if you submitted all the jobs at once, they would mess each other up. The build can't happen in parallel in the same directory.

I really do want to figure out how to do all the builds in parallel. If I do 10 builds on Anvil, it takes hours. I will probably add that in the future as an optional feature where it creates a worktree for each build. That has its own serious overhead but would save time for situations with a lot of builds.

@xylar
Copy link
Collaborator Author

xylar commented Apr 28, 2022

@sbrus89, I added more documentation (including docstrings, oops!). I also added the --submit flag but I'm not 100% sure the logic for detecting the suite name will be correct. I'll test as I have time (but not at 10 pm). If you have time to test, feel free. But be warned that bugs await you if past experience is any guide...

@xylar xylar force-pushed the add_matrix_build branch 2 times, most recently from c7106c3 to 3a95262 Compare April 29, 2022 06:18
@xylar xylar force-pushed the add_matrix_build branch from 3a95262 to 06b0e8b Compare April 29, 2022 07:06
@xylar
Copy link
Collaborator Author

xylar commented Apr 29, 2022

@sbrus89 the --submit flag is working. I added a sanity check to make sure the file exists.

I think it might be necessary to change matrix.log to something that includes the compass environment name. But I think it's generally too messy to run more than one matrix in the same compass branch. If you want to run a second one, I would strongly advise creating a new compass worktree and conda environment name.

@xylar xylar merged commit 6ab6483 into MPAS-Dev:master Apr 29, 2022
@xylar xylar deleted the add_matrix_build branch April 29, 2022 09:24
@sbrus89
Copy link
Collaborator

sbrus89 commented Apr 29, 2022

@xylar, I tried out the --submit option at the very end of yesterday and checked this morning to see that all the matrix jobs ran. This is super cool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request utility Utility script(s) outside of the compass package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants