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 scripts for using git bisect #354

Merged
merged 4 commits into from
Oct 13, 2022

Conversation

xylar
Copy link
Collaborator

@xylar xylar commented Apr 8, 2022

A config file determines the "good" and "bad" commits, and provides the paths to the MPAS code, the load script, etc. It
also provides the commands for building the code, setting up the test case(s) and running them.

A driver script sets things up and makes the calls to git bisect, passing along the utils/bisect/bisect_step.py script for running each step of the bisection.

The utils/bisect/bisect_step.py script updates and builds the MPAS code, then sets up the compass test case(s) and runs compass.

@xylar xylar added the enhancement New feature or request label Apr 8, 2022
@xylar xylar requested a review from sbrus89 April 8, 2022 18:36
@xylar xylar self-assigned this Apr 8, 2022
@xylar
Copy link
Collaborator Author

xylar commented Apr 8, 2022

@sbrus89, I believe this is working now. It is ready for you to test and review. I don't plan to document it beyond utils/bisect/README.md unless you would like it to be in the main documentation.

@sbrus89
Copy link
Collaborator

sbrus89 commented Apr 8, 2022

@xylar, very cool! I'll take a look.

@xylar xylar force-pushed the add_compass_bisect_script branch 4 times, most recently from 0f1f880 to ceab314 Compare April 8, 2022 20:48
@xylar
Copy link
Collaborator Author

xylar commented Apr 8, 2022

I've been trying to clean this up a bit (e.g. make sure the make output doesn't overwhelm everything else). I'll re-test tomorrow or Monday.

@xylar
Copy link
Collaborator Author

xylar commented Apr 9, 2022

Using this approach, I was able to trace the unexpected non-BFB changes in baroclinic channel runs in #326 to E3SM-Project/E3SM#4752. That's good to know but not concerning.

@xylar xylar force-pushed the add_compass_bisect_script branch 2 times, most recently from 6ea1210 to a64d918 Compare April 18, 2022 08:18
@xylar
Copy link
Collaborator Author

xylar commented Apr 19, 2022

@sbrus89, please don't forget to review this when you can.

@sbrus89
Copy link
Collaborator

sbrus89 commented Apr 19, 2022

@xylar, will do.

@xylar xylar added the utility Utility script(s) outside of the compass package label Apr 22, 2022
@sbrus89
Copy link
Collaborator

sbrus89 commented Apr 22, 2022

@xylar, I'm seeing a e3sm_hash_Obtaining file: directory showing up in my work_base directory. It seems like it remains empty. Is this intended?

@sbrus89
Copy link
Collaborator

sbrus89 commented Apr 22, 2022

Now I'm not seeing it. Probably something on my end.

@xylar xylar force-pushed the add_compass_bisect_script branch 2 times, most recently from 4030e9d to 9a8456f Compare May 4, 2022 22:57
utils/bisect/bisect.py Outdated Show resolved Hide resolved
utils/bisect/bisect.py Outdated Show resolved Hide resolved
utils/bisect/README.md Outdated Show resolved Hide resolved
utils/bisect/bisect.py Outdated Show resolved Hide resolved
utils/bisect/example.cfg Outdated Show resolved Hide resolved
utils/bisect/example.cfg Outdated Show resolved Hide resolved
@xylar xylar force-pushed the add_compass_bisect_script branch from 4123980 to a45d4f5 Compare July 5, 2022 10:11
@xylar xylar force-pushed the add_compass_bisect_script branch from a45d4f5 to 2b19d91 Compare July 6, 2022 12:53
A config file determines the "good" and "bad" commits, and
provides the paths to the MPAS code, the laod script, etc.  It
also provides the commands for building the code, setting up the
test case(s) and running them.

A driver script sets things up and makes the calls to `git bisect`,
passing along the `bisect/bisect_step.py` script for running each
step of the bisection.

The `bisect/bisect_step.py` script updates and builds the MPAS
code, then sets up the compass test case(s) and runs compass.
@xylar
Copy link
Collaborator Author

xylar commented Oct 6, 2022

@sbrus89, any chance of giving this a look in the next few weeks? It would be nice to have it in.

@sbrus89
Copy link
Collaborator

sbrus89 commented Oct 6, 2022

@xylar, sure I'll put it on my todo list.

@xylar
Copy link
Collaborator Author

xylar commented Oct 7, 2022

@sbrus89, I'm not sure what changed but I'm not having any luck with this. My runs just hang when I try to run on an interactive node and they fail in ways I haven't figured out yet when I try to run a job.

@xylar
Copy link
Collaborator Author

xylar commented Oct 7, 2022

Oh, I take that back. The job script worked fine, it just returned an error code for reasons I don't understand. It seems like it found the bad commit and then tried to run again or something. I need to investigate sometime but don't have time right now.

utils/bisect/bisect.py Outdated Show resolved Hide resolved
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.

I tested this and it works great! I'm a huge fan of this capability. I have a couple minor suggestions pointed out above.

The only other comment I have is it may be convenient to create a summary log file that just prints the commit information along with the PASS/FAIL status of each of the commits that is checked-out in the process. I think that is very optional though...

utils/bisect/bisect.py Outdated Show resolved Hide resolved
utils/bisect/bisect_step.py Outdated Show resolved Hide resolved
utils/bisect/example.cfg Outdated Show resolved Hide resolved
@xylar
Copy link
Collaborator Author

xylar commented Oct 13, 2022

The only other comment I have is it may be convenient to create a summary log file that just prints the commit information along with the PASS/FAIL status of each of the commits that is checked-out in the process. I think that is very optional though...

@sbrus89, I don't feel like I have time to do that right now. I would welcome a follow-up PR to add this, though!

Could you take a look and see if I've addressed your suggestions? I think so. If so, I'll merge after CI has finished unless you want to run another test.

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.

Approved. Thanks very much, @xylar!

@xylar xylar merged commit a64342f into MPAS-Dev:master Oct 13, 2022
@xylar
Copy link
Collaborator Author

xylar commented Oct 13, 2022

@sbrus89, great, thanks!

@xylar xylar deleted the add_compass_bisect_script branch October 13, 2022 15:16
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