Skip to content

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented May 13, 2024

The renaming is done so that the fix here can take effect on PR runs via the "CI Fix" mechanism (as changes to the workflow files themselves cannot take effect).

  • Moreover, making arm64 the default platform for macOS makes sense because all new macOS machines use this platform.

And update by running .github/workflows/conda-lock-update.py for all architectures.

Author: @mkoeppe, @gmou3

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

Copy link

github-actions bot commented May 13, 2024

Documentation preview for this PR (built with commit b560f4d; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 13, 2024

  File "sage/algebras/quatalg/quaternion_algebra_cython.pyx", line 1, in init sage.algebras.quatalg.quaternion_algebra_cython
  File "sage/matrix/matrix_integer_dense.pyx", line 1, in init sage.matrix.matrix_integer_dense
ImportError: dlopen(/Users/runner/work/sage/sage/src/sage/libs/linbox/linbox_flint_interface.cpython-311-darwin.so, 0x0002): symbol not found in flat namespace '__ZN6FFPACK8CharPolyIN6Givaro8Poly1DomINS1_7ModularIddvEENS1_5DenseEEEEERNT_7ElementERKS7_S9_mNS7_8Domain_t11Element_ptrEmRNSC_8RandIterENS_19FFPACK_CHARPOLY_TAGEm'

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 13, 2024

  In file included from /Users/runner/miniconda3/envs/sage/include/giac/giac.h:5:
  In file included from /Users/runner/miniconda3/envs/sage/include/giac/poly.h:26:
  /Users/runner/miniconda3/envs/sage/include/giac/monomial.h:341:10: error: no template named 'pointer_to_binary_function' in namespace 'std'
      std::pointer_to_binary_function < const monomial<T> &, const monomial<T> &, bool> strictly_greater ;
      ~~~~~^
  /Users/runner/miniconda3/envs/sage/include/giac/monomial.h:342:28: error: no template named 'pointer_to_binary_function' in namespace 'std'
      sort_helper(const std::pointer_to_binary_function < const monomial<T> &, const monomial<T> &, bool> is_strictly_greater):strictly_greater(is_strictly_greater) {};
                        ~~~~~^
  /Users/runner/miniconda3/envs/sage/include/giac/monomial.h:343:41: error: no member named 'ptr_fun' in namespace 'std'
      sort_helper():strictly_greater(std::ptr_fun<const monomial<T> &, const monomial<T> &, bool>(m_lex_is_strictly_greater<T>)) {};
                                     ~~~~~^
  /Users/runner/miniconda3/envs/sage/include/giac/monomial.h:343:49: error: expected expression
      sort_helper():strictly_greater(std::ptr_fun<const monomial<T> &, const monomial<T> &, bool>(m_lex_is_strictly_greater<T>)) {};
                                                  ^
  /Users/runner/miniconda3/envs/sage/include/giac/monomial.h:343:70: error: expected expression
      sort_helper():strictly_greater(std::ptr_fun<const monomial<T> &, const monomial<T> &, bool>(m_lex_is_strictly_greater<T>)) {};
                                                                       ^
  /Users/runner/miniconda3/envs/sage/include/giac/monomial.h:343:95: error: expected '(' for function-style cast or type construction
      sort_helper():strictly_greater(std::ptr_fun<const monomial<T> &, const monomial<T> &, bool>(m_lex_is_strictly_greater<T>)) {};
                                                                                            ~~~~^
  /Users/runner/miniconda3/envs/sage/include/giac/monomial.h:680:18: error: no template named 'pointer_to_binary_function' in namespace 'std'
               const std::pointer_to_binary_function < const monomial<T> &, const monomial<T> &, bool> m_is_strictly_greater
                     ~~~~~^
  /Users/runner/miniconda3/envs/sage/include/giac/monomial.h:729:44: error: no template named 'pointer_to_binary_function' in namespace 'std'
      typedef std::map< index_t,T,const std::pointer_to_binary_function < const index_m &, const index_m &, bool> > application;
                                        ~~~~~^
  /Users/runner/miniconda3/envs/sage/include/giac/monomial.h:730:30: error: no type named 'ptr_fun' in namespace 'std'
      application produit(std::ptr_fun(is_strictly_greater));
                          ~~~~~^
  /Users/runner/miniconda3/envs/sage/include/giac/monomial.h:737:14: error: 'application' (aka 'int') is not a class, namespace, or enumeration
      typename application::iterator prod_it,prod_itend;
               ^
  /Users/runner/miniconda3/envs/sage/include/giac/monomial.h:754:29: error: use of undeclared identifier 'prod_itend'; did you mean 'prod_it'?
      prod_it=produit.begin(),prod_itend=produit.end();

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 13, 2024

That's with giac 1.9.0.21 h1c96721_1 conda-forge

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 13, 2024

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 13, 2024

  building 'sage.matroids.graphic_matroid' extension
  /home/runner/miniconda3/envs/sage/bin/x86_64-conda-linux-gnu-cc -Wno-unused-result -Wsign-compare -DNDEBUG -fwrapv -O2 -Wall -fPIC -O2 -isystem /home/runner/miniconda3/envs/sage/include -fPIC -O2 -isystem /home/runner/miniconda3/envs/sage/include -march=nocona -mtune=haswell -ftree-vectorize -fPIC -fstack-protector-strong -fno-plt -O2 -ffunction-sections -pipe -isystem /home/runner/miniconda3/envs/sage/include -DNDEBUG -D_FORTIFY_SOURCE=2 -O2 -isystem /home/runner/miniconda3/envs/sage/include -fPIC -Isage/data_structures -I/home/runner/miniconda3/envs/sage/lib/python3.9/site-packages/cysignals -Isage/cpython -I/home/runner/work/sage/sage/src -I/home/runner/miniconda3/envs/sage/lib/python3.9/site-packages/numpy/core/include -I/home/runner/miniconda3/envs/sage/include/python3.9 -I/home/runner/miniconda3/envs/sage/include/python3.9 -c sage/matroids/graphic_matroid.c -o /tmp/tmpj1q6x7la.build-temp/sage/matroids/graphic_matroid.o
  sage/matroids/graphic_matroid.c: In function '__pyx_f_4sage_8matroids_15graphic_matroid_14GraphicMatroid__has_minor':
  sage/matroids/graphic_matroid.c:23551:234: error: macro "minor" passed 3 arguments, but takes just 1
  23551 |       __pyx_t_2 = ((struct __pyx_vtabstruct_4sage_8matroids_15graphic_matroid_GraphicMatroid *)__pyx_v_self->__pyx_base.__pyx_vtab)->__pyx_base.minor(((struct __pyx_obj_4sage_8matroids_7matroid_Matroid *)__pyx_v_self), 0, &__pyx_t_21); if (unlikely(!__pyx_t_2)) __PYX_ERR(0, 610, __pyx_L1_error)
        |                                                                                                                                                                                                                                          ^
  In file included from /home/runner/miniconda3/envs/sage/x86_64-conda-linux-gnu/sysroot/usr/include/sys/types.h:226,
                   from /home/runner/miniconda3/envs/sage/x86_64-conda-linux-gnu/sysroot/usr/include/stdlib.h:320,
                   from /home/runner/miniconda3/envs/sage/include/python3.9/Python.h:34,
                   from sage/matroids/graphic_matroid.c:50:
  /home/runner/miniconda3/envs/sage/x86_64-conda-linux-gnu/sysroot/usr/include/sys/sysmacros.h:66: note: macro "minor" defined here
     66 | # define minor(dev) gnu_dev_minor (dev)
        |

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 13, 2024

@gmou3 Here we have run in a strange symbol clash from the Cythonization of GraphicMatroid. See https://stackoverflow.com/questions/22240973/major-and-minor-macros-defined-in-sys-sysmacros-h-pulled-in-by-iterator; would you be able to work around it?

@vbraun
Copy link
Member

vbraun commented May 17, 2024

Shouldn't the CI then work on this ticket if this ticket fixes it? Because it seems like @mkoeppe canceled the run. Should it be this way?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 17, 2024

@vbraun This fixes the "Build&Test using conda": See the green checkmarks.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 17, 2024

@vbraun I've now updated the branch protection rule to make the "Expected -- Waiting for status to be reported" disappear (for conda)

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 18, 2024

Thanks, Volker.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented May 20, 2024

Tests for the 9 platforms running at https://github.com/mkoeppe/sage/actions/runs/9154171460

vbraun pushed a commit to vbraun/sage that referenced this pull request May 24, 2024
sagemathgh-38033: Avoid conflict with macro `minor`
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Author: @gmou3

Reviewer: @mkoeppe

- Split out from sagemath#37998

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38033
Reported by: Matthias Köppe
Reviewer(s):
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 3, 2024
sagemathgh-38034: `sage.libs.giac`: Compile with std=c++11
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Fixes
sagemath#37998 (comment)

- Split out from sagemath#37998

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38034
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 5, 2024
sagemathgh-38034: `sage.libs.giac`: Compile with std=c++11
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Fixes
sagemath#37998 (comment)

- Split out from sagemath#37998

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38034
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 6, 2024

@vbraun Let's get this in please.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 6, 2024

Labels are being manipulated again here.

@saraedum
Copy link
Member

saraedum commented Jun 6, 2024

Labels are being manipulated again here.

I suppose that @tobiasdiez is being blocked by you so he cannot state what he would like to change in this PR?

Apart from that, it would seem to me that he's entitled to set a PR to "needs work" and request changes. Tobias, maybe you can try to share what you would like to change by some other means?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 6, 2024

He previously communicated in the thread https://groups.google.com/g/sage-devel/c/tw6J8vU7IY0

@saraedum
Copy link
Member

saraedum commented Jun 6, 2024

Apparently he did not feel that his remarks were addressed sufficiently but I can only speculate.

vbraun pushed a commit to vbraun/sage that referenced this pull request Jun 8, 2024
sagemathgh-38034: `sage.libs.giac`: Compile with std=c++11
    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

Fixes
sagemath#37998 (comment)

- Split out from sagemath#37998

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] The title is concise and informative.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation and checked the documentation
preview.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->
    
URL: sagemath#38034
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
@vbraun vbraun closed this in 5b3e98e Jun 9, 2024
@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 25, 2024

@vbraun I've redone the branch so that the conda package update from #35396 is reflected.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 25, 2024

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 26, 2024

@kwankyu
Copy link
Collaborator

kwankyu commented Jun 26, 2024

False facts. What Tobias cites as a "test run before this commit" is actually a test run with an earlier version of this PR merged as a CI Fix.

Confirmed. "Test run before this commit" has the same failures but in the baseline set from this PR merged as a CI Fix.

Tobias' argument (no upgrade that introduces new failures) is sound but based on false facts.

@kwankyu
Copy link
Collaborator

kwankyu commented Jun 26, 2024

Is this

sage -t --random-seed=147712982467446710824553287018105912799 src/sage/libs/singular/function.pyx  # 3 doctests failed

new?

@kwankyu
Copy link
Collaborator

kwankyu commented Jun 26, 2024

As we are here, may I ask (not a thing to do here): Could we put all these environment-xxx files into, say conda directory to clean up the sage root directory, and recommend

conda env create --file ./conda/environment-3.11-linux.yml --name sage-build

in the developer guide?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 26, 2024

some restructuring like this could be a good idea, but not for this PR

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 26, 2024

Is this

sage -t --random-seed=147712982467446710824553287018105912799 src/sage/libs/singular/function.pyx  # 3 doctests failed

new?

I think this is another sporadic failure, which I've probably seen before.
It does not show in the set of tests https://github.com/mkoeppe/sage/actions/runs/9672092917

@kwankyu
Copy link
Collaborator

kwankyu commented Jun 26, 2024

Another question. This is also not to be done here:

It seems that the script .github/workflows/conda-lock-update.py is a utility for humans. Why is it in .github/workflows?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 26, 2024

It was said at the time that maybe one day these lock files would be updated automatically by a GH Actions workflow.

@kwankyu
Copy link
Collaborator

kwankyu commented Jun 26, 2024

Regarding

The renaming of the macos lock files are now inconsistent with the naming of the linux lock files. We should not orient ourselves on what github actions use as the default, but what is the most common system - these files are primarily intended to be used by users, and not for the ci.

As far as I can see, there is nothing "oriented by github actions" in naming lock files. But the comment may be to the old version of this PR...

@kwankyu
Copy link
Collaborator

kwankyu commented Jun 26, 2024

It was said at the time that maybe one day these lock files would be updated automatically by a GH Actions workflow.

Yes, I thought something like it. Perhaps ci-conda.yml may run the script and deliver the diffs of updated lock files as an artifact or as annotations...

Copy link
Collaborator

@kwankyu kwankyu left a comment

Choose a reason for hiding this comment

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

I am positive to this PR. We need this PR, ASAP to fix conda ci.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jun 26, 2024

Thanks.

@vbraun vbraun merged commit 82bc2c3 into sagemath:develop Jun 27, 2024
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.

CI Conda is broken because macos-latest is now Apple Silicon
6 participants