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

.github/workflows/ci-sage.yml: New #1166

Closed
wants to merge 2 commits into from
Closed

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Jul 2, 2022

This is an updated version of the CI workflow previously contributed to one of the releases branches.

It reveals a number of portability issues:

  1. On ubuntu-bionic-standard: https://github.com/wbhart/flint2/runs/7164118857?check_suite_focus=true
get_set_coeff_fmpq_monomial....FAIL
check transitivity
i = 365
../Makefile.subdirs:107: recipe for target '../build/fmpq_mpoly/test/t-cmp_RUN' failed
make[4]: *** [../build/fmpq_mpoly/test/t-cmp_RUN] Aborted (core dumped)
  1. On cygwin: https://github.com/wbhart/flint2/runs/7164118968?check_suite_focus=true
/bin/sh: line 3: 57749 Segmentation fault      (core dumped) build/interfaces/test/t-NTL-interface.exe
  [flint-git]   make[3]: *** [Makefile:248: check] Error 139
  1. On macos-homebrew: https://github.com/wbhart/flint2/runs/7164115814?check_suite_focus=true
    arb 2.22.1 fails to build with this version of flint

(Some of the macOS builds fail for reasons unrelated to FLINT. I'll clean this up in a follow-up PR when we have done a corresponding update in Sage - https://trac.sagemath.org/ticket/32570)

@albinahlback
Copy link
Collaborator

I'm not talking for Fredrik, but I think it would be very beneficial for us to have a Sage CI. However, I believe we cannot have 100 runners as that would be eat up our runner capacity. Would it be possible to reduce this down to two-three runners only?

And regarding NTL, it has basically not been touched in 8 years as well as not tested, so I would, unfortunately, presume that it is dead.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2023

regarding NTL, it has basically not been touched in 8 years as well as not tested, so I would, unfortunately, presume that it is dead.

In Sage, we do make use of the FLINT-NTL interface, so at least a part of it undergoes regular testing.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Mar 24, 2023

I believe we cannot have 100 runners as that would be eat up our runner capacity. Would it be possible to reduce this down to two-three runners only?

Sure, the number of parallel runners used, as well as the tested system configurations, are of course configurable.
I'll be happy to update the PR if there's sufficient interest.

@albinahlback
Copy link
Collaborator

albinahlback commented Mar 24, 2023

What's the timescale of the jobs? Preferable they would be under 30 min, but I suppose that under one hour is also fine.

Edit: I believe we can cache previous builds in order for the Sage actions to be faster.

@albinahlback
Copy link
Collaborator

I am going to close this -- feel free to open it again.

However, note that we do not have an unlimited number of workers available, hence I think we realistically can only take a total of 30 minutes worker time for this.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 29, 2024

I'll open a PR with an updated version that uses Sage's reusable workflows.

@edgarcosta
Copy link
Member

Flint already struggles quite a lot with resources for CI.
Upstream compilation of sage is key, and we should figure out how to test without significantly reducing the resources for the rest of the tests

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jan 29, 2024

You probably mean "downstream". Sage is downstream of FLINT. And yes, with the reusable workflows it can be configured so that we use the prebuilt Docker images of Sage and only recompile the libraries that depend on FLINT.

Resource use is easily regulated by simply deciding when to run the full portability tests. Obviously one does not want to run them on every PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants