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

CI: use macOS ARM-based runner #3395

Merged
merged 1 commit into from
Feb 15, 2024
Merged

CI: use macOS ARM-based runner #3395

merged 1 commit into from
Feb 15, 2024

Conversation

nilason
Copy link
Contributor

@nilason nilason commented Feb 2, 2024

Replace the macOS runner with the new ARM64 based one.

Two tests are not passing, so I put them on the exclusion list.

This will cut the CI run time with approximately 50 %!

@nilason nilason added this to the 8.4.0 milestone Feb 2, 2024
@nilason nilason self-assigned this Feb 2, 2024
@github-actions github-actions bot added the CI Continuous integration label Feb 2, 2024
@petrasovaa
Copy link
Contributor

Two tests are not passing, so I put them on the exclusion list.

Is there a way to see why they are not passing, specifically I am wondering about r.horizon test.

@nilason
Copy link
Contributor Author

nilason commented Feb 2, 2024

Two tests are not passing, so I put them on the exclusion list.

Is there a way to see why they are not passing, specifically I am wondering about r.horizon test.

I planned to create tickets for them, but to give you a head start:

test_r_horizon

./raster/r.horizon – test_r_horizon

❌ Test failed

Test	test_r_horizon
Testsuite	./raster/r.horizon
Test file	./raster/r.horizon/testsuite/test_r_horizon.py
Status	FAILED
Return code	1
Number of tests	6
Successful tests	5
Failed tests	1
Percent successful	83%
Test duration	0:00:21.037551
Tested modules	r.horizon
Supplementary files

standard output (stdout)
standard error output (stderr)
Standard error output (stderr)

.F....
======================================================================
FAIL: test_point_mode_multiple_direction_artificial (__main__.TestHorizon.test_point_mode_multiple_direction_artificial)
Test mode with 1 point and multiple directions with artificial surface
----------------------------------------------------------------------
Traceback (most recent call last):
  File "raster/r.horizon/testsuite/test_r_horizon.py", line 159, in test_point_mode_multiple_direction_artificial
    self.assertMultiLineEqual(first=ref4, second=stdout)
  File "etc/python/grass/gunittest/case.py", line 179, in assertMultiLineEqual
    return super().assertMultiLineEqual(first=first, second=second, msg=msg)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: 'azim[15 chars]ght\n0.000000,0.197017\n20.000000,0.196832\n40[327 chars]63\n' != 'azim[15 chars]ght\n360.000000,0.197017\n20.000000,0.196832\n[329 chars]63\n'
  azimuth,horizon_height
- 0.000000,0.197017
+ 360.000000,0.197017
? ++
  20.000000,0.196832
  40.000000,0.196875
  60.000000,0.196689
  80.000000,0.196847
  100.000000,0.196645
  120.000000,0.196969
  140.000000,0.196778
  160.000000,0.196863
  180.000000,0.197017
  200.000000,0.196832
  220.000000,0.196875
  240.000000,0.196689
  260.000000,0.196847
  280.000000,0.196645
  300.000000,0.196969
  320.000000,0.196778
  340.000000,0.196863


----------------------------------------------------------------------
Ran 6 tests in 20.967s
FAILED (failures=1)

<\details>

@nilason
Copy link
Contributor Author

nilason commented Feb 2, 2024

For the record: building now takes about 2.5 min, the run in total some 33 min. Compared to ~7 min and ~79 min.

@echoix
Copy link
Member

echoix commented Feb 2, 2024

The extra core (3 core instead of 2), and the fact that they are fast, seems to help a lot! Even if our tests aren't really parallelized, it's still faster!

@nilason
Copy link
Contributor Author

nilason commented Feb 2, 2024

The extra core (3 core instead of 2), and the fact that they are fast, seems to help a lot! Even if our tests aren't really parallelized, it's still faster!

I was literally blown away, the first time I compiled GRASS with a M1 Pro (8-core). Compilation can take advantage of both parallelisation and the sheer speed of Apple's ARM architecture.

@echoix
Copy link
Member

echoix commented Feb 3, 2024

I'm kinda suprised that only two tests failed. Especially for the r3.flow, the numerical differences are quite strange. What do these two modules do differently (or correctly in tests) from the others? If it was only architectural differences, I would have expected way more.

@nilason nilason added the macOS macOS specific label Feb 15, 2024
Copy link
Member

@echoix echoix left a comment

Choose a reason for hiding this comment

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

Let's try with this, it'll speed up the feedback loop a fair bit, and is closer to what macOS users would use now. Intel Macs are getting older and older

@nilason
Copy link
Contributor Author

nilason commented Feb 15, 2024

Let's try with this, it'll speed up the feedback loop a fair bit, and is closer to what macOS users would use now. Intel Macs are getting older and older

I agree. And the only "regression" on tests is reported with #3398 (which seems more like an "imperfect" test rather than actual bug).

@nilason nilason merged commit 05b9b03 into OSGeo:main Feb 15, 2024
26 checks passed
@echoix
Copy link
Member

echoix commented Feb 15, 2024

Since the regression is already flagged elsewhere, I'm fine going forward with this. That's for your PR! You were the one that made me know about that new change, usually I read GitHub's changelog blog every couple of weeks, and didn't know about it when your PR landed.

jadenabrams100 pushed a commit to ncsu-csc472-spring2024/grass-CI-playground that referenced this pull request Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous integration macOS macOS specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants