4. Hyper-parameter tuning (extension with XGBoost)

[jvmead]

an extension to the ML lessons, picking up where k-fold left off
currently runs into problems with the CI timing out, presumably due to long running time?

https://colab.research.google.com/
Notebook real time --- 1051.062778711319 seconds ---
Notebook CPU time --- 728.3288249999999 seconds ---

The successfully executed notebook with plots generated using full stats may be found here:
https://colab.research.google.com/drive/1I34R8eCck3wo1YX54WllUKJw9KdoID8Y

[chrisburr]

It looks like there are some more imports to fix

[jvmead]

@chrisburr down to <20mins on colab, is there any leeway we can get on the CI?

[jvmead]

@chrisburr unsure of what the issue is, and how to proceed with the last step:
test $TRAVIS_PULL_REQUEST == "false" && test $TRAVIS_BRANCH == "master" && starterkit_ci deploy

[chrisburr]

You can ignore that, it’s the bit that uploads it to the website but we can fix that later

[jonas-eschle]

@jvmead finally unstalled it, the CI works again etc, back to this!
Why is the ci.yaml needed? We do already have the building?

And can we rename it to 33 maybe? So that all the 3 notebooks are classification/BDT/ML?

[jvmead]

@jonas-eschle Hi! Great to hear :) I've renamed the lesson notebook as you suggest.

ci.yaml might just be a vestige from attempts to get this lesson to play nice with the CI (like extending runtime limits etc. perhaps, by switching back from sphinx to gitbook or travis) I can't quite remember - it's been quite a while. The .github directory it resides in doesn't even exist on main.

P.S.
As I may have mentioned in previous comments, there have since been some additions to my local version of the notebook - I just didn't want to throw a spanner in the works as you fought to get this issue resolved. If you think the additions are worth including then once you're happy everything on your side with the current version I'll only then look at committing the latest. They're mostly just a warning against overtraining / problems which could be introduced by how you define your classes (it seems the 'optimal' tuned model has learned the invariant mass window used to define Sig vs Bkg in training - a fun albeit unintentional cautionary example / reason to use same-sign background MC within the mass window).

[jonas-eschle]

Also, the ci.yaml should then be removed, right?

[jonas-eschle]

For the additions, I would suggest to get this merged, and then quickly make a new PR with the additions?
I think it just needs to be cleaned up a bit (see also review comments) but other than that, it looks good to me

(sidenote, for future reference: name your branch more meaningful, i.e. paramtune, bdttutorial_hyper or whatever, not simply Patch-1 (which is a github default, I am aware))

[jonas-eschle]

Okay, sounds good. Can you remove the files and undo the changes?

The .github directory it resides in doesn't even exist on main.

Not quite, it does, but not the fork you created (maybe use the "sync fork" button in the repo).

Btw, if you encounter technical difficulties along the way now, let me know (and if it's easier to add the changes now, we can too)

[jvmead]

my bad! I didn't mean to hit ready for review yet, I'm just reverting the changes to lesson 10 and 11 now

[jonas-eschle]

Okay, looks good to me, lgtm once the CI passes

[jonas-eschle]

@jvmead , there seems to be an issue in the notebook, can you check and verify that you can run it locally?

[jvmead]

Should be all good now!

[welcome]

Congrats on merging your first pull request 🎉! We greatly appreciate it.
You might be eligible to be added to the HSF Training Community page (see the instructions on the page for how to create a profile). If you already have created a profile previously, make sure you're also added to the current year. If this repository features a list of contributors at the bottom of the readme, you might also be eligible to add yourself there.

[klieret]

Congrats on merging this big PR! Btw, what the bot just commented is right: We'd be happy to add you to the community page @jvmead and @jonas-eschle :)

[jvmead]

Hi @klieret,
I'd be happy to be listed! I've added an .md file and made a merge request if that's still all that's necessary. I did notice just now that this lesson's inclusion in the advanced python contents page isn't quite right. Can this be edited trivially or should another fork be made?

[klieret]

You can also just send me the filled out template from https://hepsoftwarefoundation.org/howto-profile.html :)

For fixing the inclusion: Probably you can simply start a new branch on your fork and create a PR from there, right?
(or you can leave a comment here for someone to fix it, I guess)

[jvmead]

where is the contents page? hfs.github.io? or does it full straight from the markdown headings on the ipynb?