8000 patching gfan by edgarcosta · Pull Request #40065 · sagemath/sage · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

patching gfan #40065

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
May 11, 2025
Merged

patching gfan #40065

merged 1 commit into from
May 11, 2025

Conversation

edgarcosta
Copy link
Member

This fixes failed build on OSX:

[gfan-0.6.2.p1] [spkg-install] In file included from src/gfanlib_zcone.cpp:8:
[gfan-0.6.2.p1] [spkg-install] In file included from src/gfanlib_zcone.h:11:
[gfan-0.6.2.p1] [spkg-install] src/gfanlib_matrix.h:123:18: error: no member named 'vectormultiply' in 'Matrix<typ>'
[gfan-0.6.2.p1] [spkg-install]   123 |         ret[i]=a.vectormultiply(b.column(i));
[gfan-0.6.2.p1] [spkg-install]       |                ~ ^
[gfan-0.6.2.p1] [spkg-install] 1 error generated.
[gfan-0.6.2.p1] [spkg-install] make[3]: *** [src/gfanlib_zcone.o] Error 1

@fchapoton already implemented this fix in Singular fchapoton/Singular@d3f7343

Boring details

this comments out a method

  friend Matrix operator*(const Matrix& a, const Matrix& b)
    {
      assert(a.width==b.height);
      Matrix ret(b.width,a.height);
      for(int i=0;i<b.width;i++)
        ret[i]=a.vectormultiply(b.column(i));
      return ret.transposed();
    }

which depends on a method already commented out (that also doesn't parse as valid code)

  /*IntegerVector vectormultiply(IntegerVector const &v)const
    {
      assert(v.size()==width);
      IntegerVector ret(height);
      for(int i=0;i<height;i++)
        ret[i]=dot(rows[i],v);
      return ret;
    }*/

@edgarcosta edgarcosta requested a review from fchapoton May 7, 2025 19:23
@github-actions GitHub Actions
Copy link
github-actions bot commented May 7, 2025

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

@fchapoton
Copy link
Contributor

are you sure that it is not used anywhere ?

@edgarcosta
Copy link
Member Author

The things I am confident about:

  1. the method IntegerVector vectormultiply(IntegerVector const &v)const is commented out and is not valid code
  2. matrix.h also has an implementation of 1., but this is header is not included in in gfanlib_matrix.h
  3. Hence, the method Matrix operator*(const Matrix& a, const Matrix& b) depends on 1.
  4. with the fix I can build gfan.

Does this give a proof? nope, templating is hard, but I am quite confident.

fchapoton
fchapoton approved these changes May 8, 2025
View reviewed changes
Copy link
Contributor
@fchapoton fchapoton left a comment

Choose a reason for hiding this comment

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

ok, let's move on

@edgarcosta
Copy link
Member Author

Thank you.

@vbraun vbraun merged commit 19e94c2 into sagemath:develop May 11, 2025
26 of 28 checks passed
Sign up for 5FAE free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fchapoton fchapoton fchapoton approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@edgarcosta @fchapoton @vbraun
0