Skip to content

Use sig_block to avoid conflict signal handle#41209

Merged
vbraun merged 7 commits intosagemath:developfrom
cxzhong:patch-9
Feb 11, 2026
Merged

Use sig_block to avoid conflict signal handle#41209
vbraun merged 7 commits intosagemath:developfrom
cxzhong:patch-9

Conversation

@cxzhong
Copy link
Copy Markdown
Contributor

@cxzhong cxzhong commented Nov 20, 2025

Improve #40980 , there are other two functions are also needs sig_block. It is compatible with #40980

📝 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

#40980

@cxzhong cxzhong marked this pull request as ready for review November 20, 2025 12:18
@cxzhong
Copy link
Copy Markdown
Contributor Author

cxzhong commented Nov 21, 2025

@user202729 Can you help me review this. It is a minor improvement

@user202729
Copy link
Copy Markdown
Contributor

add a test that would have failed without the change.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Dec 7, 2025

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

@cxzhong
Copy link
Copy Markdown
Contributor Author

cxzhong commented Dec 18, 2025

I will wait for #40980 merge

@cxzhong
Copy link
Copy Markdown
Contributor Author

cxzhong commented Dec 25, 2025

@user202729 Please review this. Thank you.

Comment thread src/sage/calculus/integration.pyx Outdated
(1.418, 0.01)
(1.418, 0.01)

Ensure :issue:`30379` is fixed for monte_carlo_integral (c_monte_carlo_f)::
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Format function name as :func:`...`​. Same for above.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And please mark the test as # long time since the CI is warning about it

@user202729
Copy link
Copy Markdown
Contributor

user202729 commented Dec 25, 2025

Otherwise, more or less correct. There's this case

sage: g(x) = function("f", evalf_func=lambda self, x, parent=None, algorithm=None: math.sqrt(x))(x)
sage: monte_carlo_integral(g, [1], [2], 100, algorithm="plain")
sage: monte_carlo_integral(g, [-1], [1], 100, algorithm="plain")

but probably not worth it to add a try/except. (if anything, I think the right behavior ought to be use a sig_error to propagate the exception upward.)

The Wrapper_rdf case might be a performance regression in case of pure functions (i.e. _n_py_constants == 0, need profiling).

Comment thread src/sage/calculus/integration.pyx Outdated
Ensure :issue:`30379` is fixed for monte_carlo_integral (c_monte_carlo_f)::

sage: def f(x, y): return gamma_inc(2, 11/5) * x * y
sage: for algo in ['plain', 'miser', 'vegas']: # abs tol 0.1
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same (long time)

@orlitzky
Copy link
Copy Markdown
Contributor

orlitzky commented Feb 2, 2026

Looks good, thanks

vbraun pushed a commit to vbraun/sage that referenced this pull request Feb 5, 2026
sagemathgh-41209: Use sig_block to avoid conflict signal handle
    
<!-- ^ 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". -->
Improve sagemath#40980 , there are other two functions are also needs
``sig_block``. It is compatible with sagemath#40980

### 📝 Checklist

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

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] 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: ... -->
sagemath#40980
    
URL: sagemath#41209
Reported by: Chenxin Zhong
Reviewer(s): Michael Orlitzky, user202729
vbraun pushed a commit to vbraun/sage that referenced this pull request Feb 7, 2026
sagemathgh-41209: Use sig_block to avoid conflict signal handle
    
<!-- ^ 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". -->
Improve sagemath#40980 , there are other two functions are also needs
``sig_block``. It is compatible with sagemath#40980

### 📝 Checklist

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

- [x] The title is concise and informative.
- [x] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [x] 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: ... -->
sagemath#40980
    
URL: sagemath#41209
Reported by: Chenxin Zhong
Reviewer(s): Michael Orlitzky, user202729
@vbraun vbraun merged commit c927506 into sagemath:develop Feb 11, 2026
19 of 21 checks passed
@cxzhong cxzhong deleted the patch-9 branch February 13, 2026 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants