ziatdinovmax / gpax

Gaussian Processes for Experimental Sciences
http://gpax.rtfd.io
MIT License
205 stars 27 forks source link

Add notebook smoke tests, QOL changes, part 2 #39

Closed matthewcarbone closed 1 year ago

matthewcarbone commented 1 year ago

Same as #38 but I actually remembered to pull from upstream first:

Just playing around/exploring the code a bit. While I was I figured I'd make a few QOL changes to the testing system:

(there will no doubt be bugs which I will fix πŸ‘)

matthewcarbone commented 1 year ago

@ziatdinovmax looks like these jobs just failed with no error messages... Doesn't even look like they ran. 😁 Any idea what's going on?

matthewcarbone commented 1 year ago

@ziatdinovmax Pretty sure I found the bug. I just did something really silly in the workflow files.

codecov-commenter commented 1 year ago

Codecov Report

Merging #39 (b5348d0) into main (6d5bc4c) will decrease coverage by 0.04%. The diff coverage is 95.23%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##             main      #39      +/-   ##
==========================================
- Coverage   95.60%   95.57%   -0.04%     
==========================================
  Files          39       39              
  Lines        3072     3072              
==========================================
- Hits         2937     2936       -1     
- Misses        135      136       +1     
Flag Coverage Ξ”
unittests 95.57% <95.23%> (-0.04%) :arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Ξ”
gpax/models/gp.py 93.08% <95.23%> (ΓΈ)

... and 1 file with indirect coverage changes

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

ziatdinovmax commented 1 year ago

Looks like the notebook tests are failing. @matthewcarbone can you please look into it?

matthewcarbone commented 1 year ago

@ziatdinovmax Yeah I think it's because I missed two notebook-specific dependencies. That's fixed now. Question is whether or not this will work anyway.

I do think notebooks should have smoke tests, but if this doesn't work I'll revert the smoke tests pieces and experiment elsewhere 😁. Once I get it right I can just open another PR.

ziatdinovmax commented 1 year ago

@matthewcarbone For some reason, I can't run the osx workflows. Given that JAX support on the Apple M series is still limited and buggy, I'm not sure how helpful they are anyway. Maybe you can remove them for now?

matthewcarbone commented 1 year ago

@ziatdinovmax yeah sure I can remove them. OSX is more expensive than Linux resource-wise, though it's odd it's been 13 hours...

Will remove them but definitely consider putting them back in the future. Note I don't believe that OSX latest == M1. Stay tuned.

matthewcarbone commented 1 year ago

@ziatdinovmax start of the new FY so I'll probably start working on this a bit more! How strict are you regarding the code coverage differences? I didn't actually change any code, I just put some new lines in for clarity 😁

ziatdinovmax commented 1 year ago

I wouldn't worry about the slight decrease in the code coverage due to basic formatting.