Enable optional custom tarball URL in unit tests (and remove separate custom tarball yaml file)

[jgabry]

closes #1130

Submission Checklist

• Run unit tests
• Declare copyright holder and agree to license (see below)

Summary

Enables an optional custom tarball URL in our main unit tests workflow so that we don't need to maintain two separate workflow files. Previously we had cmdstan-tarball-check.yaml (now deleted) for checking release candidates. Now the URL for the RC tarball can be provided to our main R-cmd-check workflow.

Copyright and Licensing

Please list the copyright holder for the work you are submitting
(this will be you or your assignee, such as a university or company):
Columbia University

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the following licenses:

• Code: BSD 3-clause (https://opensource.org/licenses/BSD-3-Clause)
• Documentation: CC-BY 4.0 (https://creativecommons.org/licenses/by/4.0/)

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.28%. Comparing base (fc0d2cf) to head (236a8da).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1132   +/-   ##
=======================================
  Coverage   87.28%   87.28%           
=======================================
  Files          14       14           
  Lines        5955     5955           
=======================================
  Hits         5198     5198           
  Misses        757      757           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[jgabry]

@WardBrian I tried what you suggested in #1130. Separately from the unit tests for this PR (which are using the latest official release), I triggered it manually to run with the release candidate using this url: https://github.com/stan-dev/cmdstan/releases/download/v2.38.0-rc1/cmdstan-2.38.0-rc1.tar.gz

It worked on Mac and Linux and all the tests passed but is failing to install CmdStan on Windows:

https://github.com/stan-dev/cmdstanr/actions/runs/20763698089/job/59624808048#step:11:41

  make: *** build: Is a directory.  Stop.

Any idea what's going on? Am I making a naive Windows mistake somewhere?

[WardBrian]

@jgabry looks like the tarballs accidentally ended up with a build folder rather than a bin folder when we fixed the version issue. We'll need to prepare new ones (cc @serban-nicusor-toptal)

[jgabry]

Ah, that did not occur to me, thanks. I can rerun this whenever there’s a new one available.

Aside from that issue this seems to be working, so we should be able to get rid of that outdated custom tarball workflow file going forward, thanks for the suggestion.

[jgabry]

Just saw you reran it, thanks

[WardBrian]

Thanks for looking into this!

Any idea what the macos-devel failures would be about? Worth looking into?

[jgabry]

Both of the Mac jobs failed this time but they both passed yesterday. My guess is that we can safely ignore this. We've been getting some transient issues on Mac on GHA that we don't get locally or with Windows or Linux. Often rerunning fixes it (rerunning now). I've been meaning to make some time to look into why we get the sporadic failures on GHA.

[jgabry]

Hmm, looks like a TBB error? https://github.com/stan-dev/cmdstanr/actions/runs/20763698089/job/59709757601#step:13:906

Any ideas why that would happen sporadically?

[WardBrian]

Am I correct that it's trying to install 2.35 in that test?

• • Installing CmdStan from https://github.com/stan-dev/cmdstan/releases/download/v2.35.0/cmdstan-2.35.0.tar.gz

I think there were some issues with TBB and newer clangs that we patched more recently. Maybe the macos runners updated recently?

[jgabry]

Yeah that specific test is testing that it can install an older version from the release URL or version number since we still allow users to do that. Do you think 2.36 is recent enough? I can bump the version number in that test.

[WardBrian]

I’m not entirely sure when the issue cropped up, unfortunately. Worth a shot?

Is there any chance to just disable that test when using a custom tarball? It wouldn’t be relevant it seems

[jgabry]

Yeah it's irrelevant when using the custom tarball. I just disabled it. I'll cancel and rerun the tests with the custom tarball. We also get sporadic errors when not using a custom tarball, so I'd like to see if we can get rid of those too, so I also bumped to 2.36 in that test which will hopefully help when there's no custom tarball.

[WardBrian]

Thanks Jonah!

[WardBrian]

so we should be able to get rid of that outdated custom tarball workflow file going forward, thanks for the suggestion.

and yes, I think that workflow file can be retired as part of this

[jgabry]

Ok great, I just deleted that file as part of this PR. And unit tests with the custom tarball are in progress.

[jgabry]

Ok so now one of the Mac jobs is passing but one is still failing. Looks like a similar or the same TBB error but from a different test that wasn't skipped. But that test that's failing now is testing that rebuild_cmdstan() works so the TBB error must come from rebuilding the RC. Here's the test that's failing:

cmdstanr/tests/testthat/test-install.R

Lines 188 to 194 in fc0d2cf

	test_that("clean and rebuild works", {
	expect_output(
	rebuild_cmdstan(),
	paste0("CmdStan v", cmdstan_version(), " built"),
	fixed = TRUE
	)
	})

The test is expecting to see "CmdStan v2.38.0 built" but instead it sees:

https://github.com/stan-dev/cmdstanr/actions/runs/20791084253/job/59713158154#step:13:1394

    │ 2 warnings and 5 errors generated.
    │ make[1]: *** [tbb_misc.o] Error 1
    │ make: *** [stan/lib/stan_math/lib/tbb/tbb.def] Error 2

I'm not sure, but I doubt this is actually a problem with the RC. All the tests on this PR are passing with the latest release, so I guess it could be the RC but it's probably just a similar sporadic failure that we see sometimes.

That said, any idea what would cause this TBB error?

[WardBrian]

It's complaining about the file version_string.ver, which is generated by this script.

Modern builds of TBB seem to have removed this entirely, so there's no easy comparison unfortunately. My first guess, if this is the kind of thing you've seen before, is that there is a parallelism bug in TBB's makefiles where something is trying to read that file before it is done being written. If you set the cores option of the rebuild_cmdstan call to 1, does it reoccur?

[jgabry]

If you set the cores option of the rebuild_cmdstan call to 1, does it reoccur?

I'll try that now and rerun everything. Thanks!

[WardBrian]

If it happens again, if you rig something up to print the contents of that file ($CMDSTAN/stan/lib/stan_math/lib/tbb/version_string.ver ) it would also help!

[jgabry]

Yeah I can do that if it keeps failing