ENH: Symbolic solver for dimension specifications. #19805

pearu · 2021-08-31T21:24:41Z

Resolves #8062

Possibly conflicting with #19388 that replaces shape with f2py_shape. Whichever PR lands last, should update the usage of shape in crackfortran.py.

…ion as warnings.warn

mattip · 2021-09-01T17:58:52Z

LGTM give 6 new alerts for this PR. Are they accurate?

This pull request introduces 6 alerts when merging 26752c3 into 0656fc4 - view on LGTM.com
new alerts:

3 for Variable defined multiple times
1 for Comparison using is when operands support __eq__
1 for Unused local variable
1 for Incomplete ordering

pearu · 2021-09-01T19:57:01Z

LGTM give 6 new alerts for this PR. Are they accurate?

The LGTM alerts were partially accurate but were certainly useful to pinpoint one real bug. Fixed the bug and warnings in the latest commit.

mattip · 2021-09-01T20:12:32Z

+1 for LGTM

numpy/f2py/crackfortran.py

numpy/f2py/symbolic.py

HaoZeke

This looks fantastic! I will recheck the issues to see if this fixes more than the one referenced here; but this is a huge step forward.

pearu · 2021-09-03T19:07:35Z

scipy builds and all its tests pass with this PR.

charris · 2021-09-17T22:44:07Z

@pearu Could you add a release note for this in doc/release/upcoming_changes? This is probably a new_feature or improvement.

melissawm

Very tiny typos otherwise this looks great, thanks @pearu !

melissawm · 2021-09-21T21:20:12Z

numpy/f2py/tests/test_crackfortran.py

+
+
+class TestDimSpec(util.F2PyTest):
+    """This test site tests various expressions that are used as dimension


Suggested change

"""This test site tests various expressions that are used as dimension

"""This test suite tests various expressions that are used as dimension

melissawm · 2021-09-21T21:23:32Z

numpy/f2py/tests/test_crackfortran.py

+    `lower` and `upper` are arbitrary expressions of input parameters.
+    The evaluation is performed in C, so f2py has to translate Fortran
+    expressions to valid C expressions (an alternative approach is
+    that a developer specifies the corresponing C expressions in a


Suggested change

that a developer specifies the corresponing C expressions in a

that a developer specifies the corresponding C expressions in a

- Spelling fixes - Release note fragment for numpygh-19805.

charris · 2021-09-26T16:06:03Z

Thanks @pearu. I've done the spelling fixes and added a release note in #19961, feel free to edit the release note. Does #19388 need fixes?

MAINT: Minor cleanups after merging gh-19805

- Spelling fixes - Release note fragment for numpygh-19805.

risicle · 2022-03-06T16:46:17Z

Hmm. Ever since commit 88f988a introduced here, in NixOS, the scikit-learn 1.0.2 kernel PCA unit tests cause crashes for us which appear to be related to a corrupted heap. This is with joblib 1.1.0, scipy 1.8.0, threadpoolctl 3.0.0, gfortran 9.3.0, gcc 10.3. Still investigating.

Edit: also tried with numpy 1.22.2 including fix #20724

melissawm · 2022-03-07T13:06:23Z

@pearu thoughts?

pearu · 2022-03-08T14:44:54Z

@risicle Can you provide a minimal reproducer of the reported crashes? The reproducer should include the f2py wrapping step as the messages there may give a hint of what goes wrong.

risicle · 2022-03-08T20:34:41Z

I'll try - it's all inside scipy's flapack_dsyevr, so I'll have to kick scipy's build process a bit.

Currently trying to convince our lapack to build with debugging symbols..

risicle · 2022-03-11T00:12:59Z

A lot of fighting with debug symbols and a crash course in fortran later, it looks like what is happening is dsyevr is being called with an M of 10, but a ISUPPZ array allocated only one element long (4 bytes).

I'll try to get scipy's f2py calls to give up their secrets next.

risicle · 2022-03-11T23:04:23Z

https://gist.github.com/risicle/d7450ec92bd89bff138ead438c28ce1c
https://gist.github.com/risicle/0e201ea432ffda2d0015a51daec88e7f

@@ -49783,33 +49783,33 @@
   capi_iwork_tmp = array_from_pyobj(NPY_INT,iwork_Dims,iwork_Rank,capi_iwork_intent,Py_None);
   if (capi_iwork_tmp == NULL) {
     PyObject *exc, *val, *tb;
     PyErr_Fetch(&exc, &val, &tb);
     PyErr_SetString(exc ? exc : _flapack_error,"failed in converting hidden `iwork' of _flapack.dsyevr to C/Fortran array" );
     npy_PyErr_ChainExceptionsCause(exc, val, tb);
   } else {
     iwork = (int *)(PyArray_DATA(capi_iwork_tmp));

   /* Processing variable z */
-  z_Dims[0]=(compute_v?MAX(0,n):0),z_Dims[1]=(compute_v?(*range=='I'?iu-il+1:MAX(1,n)):0);
+  z_Dims[0]=(compute_v ? MAX(0, n) : 0),z_Dims[1]=(compute_v ? (*range == 'I' ? 1 - il + iu : MAX(1, n)) : 0);
   capi_z_intent |= F2PY_INTENT_OUT|F2PY_INTENT_HIDE;
   capi_z_tmp = array_from_pyobj(NPY_DOUBLE,z_Dims,z_Rank,capi_z_intent,Py_None);
   if (capi_z_tmp == NULL) {
     PyObject *exc, *val, *tb;
     PyErr_Fetch(&exc, &val, &tb);
     PyErr_SetString(exc ? exc : _flapack_error,"failed in converting hidden `z' of _flapack.dsyevr to C/Fortran array" );
     npy_PyErr_ChainExceptionsCause(exc, val, tb);
   } else {
     z = (double *)(PyArray_DATA(capi_z_tmp));

   /* Processing variable isuppz */
-  isuppz_Dims[0]=(compute_v?(2*(*range=='A'||(*range=='I' && iu-il+1==n)?n:0)):0);
+  isuppz_Dims[0]=(compute_v ? 2 * (*range == 'A'||(*range == 1 + 'I' && iu - il == n) ? n : 0) : 0);
   capi_isuppz_intent |= F2PY_INTENT_OUT|F2PY_INTENT_HIDE;
   capi_isuppz_tmp = array_from_pyobj(NPY_INT,isuppz_Dims,isuppz_Rank,capi_isuppz_intent,Py_None);
   if (capi_isuppz_tmp == NULL) {
     PyObject *exc, *val, *tb;
     PyErr_Fetch(&exc, &val, &tb);
     PyErr_SetString(exc ? exc : _flapack_error,"failed in converting hidden `isuppz' of _flapack.dsyevr to C/Fortran array" );
     npy_PyErr_ChainExceptionsCause(exc, val, tb);
   } else {
     isuppz = (int *)(PyArray_DATA(capi_isuppz_tmp));

pearu added 01 - Enhancement component: numpy.f2py labels Aug 31, 2021

pearu self-assigned this Aug 31, 2021

ENH: [f2py] Symbolic solver for dimension specifications.

a131dca

pearu force-pushed the gh-8062-dimdecs branch from 131a7ac to a131dca Compare August 31, 2021 21:26

pearu added 6 commits September 1, 2021 00:31

Fix lint

d721e67

Add new module numpy.f2py.symbolic

96e63aa

Add stacklevel to warn call.

f3fd627

Handle more Expr warnings.

0458585

Fix a mistake

7b229a2

Workaround numpy.tests.test_warnings bug of treating local warn funct…

26752c3

…ion as warnings.warn

pearu requested review from HaoZeke and melissawm September 1, 2021 12:25

pearu marked this pull request as ready for review September 1, 2021 12:26

Fix bugs and warnings from LGTM report

d880dce

HaoZeke reviewed Sep 1, 2021

View reviewed changes

numpy/f2py/crackfortran.py Outdated Show resolved Hide resolved

HaoZeke reviewed Sep 1, 2021

View reviewed changes

numpy/f2py/symbolic.py Outdated Show resolved Hide resolved

HaoZeke reviewed Sep 1, 2021

View reviewed changes

numpy/f2py/symbolic.py Show resolved Hide resolved

HaoZeke approved these changes Sep 1, 2021

View reviewed changes

pearu added 2 commits September 3, 2021 19:05

Add ternary, ref, and deref expressions support.

345c9d9

Fix a bug in dependencies. Implement support for relational operations.

88f988a

pearu mentioned this pull request Sep 6, 2021

ENH: Support character and character string arrays #19388

Merged

melissawm approved these changes Sep 21, 2021

View reviewed changes

charris merged commit 4681424 into numpy:main Sep 26, 2021

charris added a commit to charris/numpy that referenced this pull request Sep 26, 2021

MAINT: Minor cleanups after merging numpygh-19805

b8e46c6

- Spelling fixes - Release note fragment for numpygh-19805.

charris mentioned this pull request Sep 26, 2021

MAINT: Minor cleanups after merging gh-19805 #19961

Merged

charris added a commit that referenced this pull request Sep 28, 2021

Merge pull request #19961 from charris/minor-fixups

ef56449

MAINT: Minor cleanups after merging gh-19805

howjmay pushed a commit to howjmay/numpy that referenced this pull request Sep 29, 2021

MAINT: Minor cleanups after merging numpygh-19805

ebcd0ee

- Spelling fixes - Release note fragment for numpygh-19805.

inducer mentioned this pull request Jan 2, 2022

BUG: [Regression 1.21.4 -> 1.22.0] f2py infers integers incorrectly from shape (-(nterms):nterms,nvcount) #20709

Closed

risicle mentioned this pull request Mar 6, 2022

python3Packages.numpy: 1.21.5 -> 1.23.1 NixOS/nixpkgs#156578

Closed

13 tasks

HaoZeke mentioned this pull request Apr 19, 2022

BUG: numpy 1.22 incorrectly infers dimensions #21362

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Symbolic solver for dimension specifications. #19805

ENH: Symbolic solver for dimension specifications. #19805

pearu commented Aug 31, 2021 •

edited

Loading

mattip commented Sep 1, 2021

pearu commented Sep 1, 2021

mattip commented Sep 1, 2021

HaoZeke left a comment

pearu commented Sep 3, 2021

charris commented Sep 17, 2021

melissawm left a comment

melissawm Sep 21, 2021

melissawm Sep 21, 2021

charris commented Sep 26, 2021

risicle commented Mar 6, 2022 •

edited

Loading

melissawm commented Mar 7, 2022

pearu commented Mar 8, 2022

risicle commented Mar 8, 2022

risicle commented Mar 11, 2022

risicle commented Mar 11, 2022



		class TestDimSpec(util.F2PyTest):
		"""This test site tests various expressions that are used as dimension

	"""This test site tests various expressions that are used as dimension
	"""This test suite tests various expressions that are used as dimension

	that a developer specifies the corresponing C expressions in a
	that a developer specifies the corresponding C expressions in a

ENH: Symbolic solver for dimension specifications. #19805

ENH: Symbolic solver for dimension specifications. #19805

Conversation

pearu commented Aug 31, 2021 • edited Loading

mattip commented Sep 1, 2021

pearu commented Sep 1, 2021

mattip commented Sep 1, 2021

HaoZeke left a comment

Choose a reason for hiding this comment

pearu commented Sep 3, 2021

charris commented Sep 17, 2021

melissawm left a comment

Choose a reason for hiding this comment

melissawm Sep 21, 2021

Choose a reason for hiding this comment

melissawm Sep 21, 2021

Choose a reason for hiding this comment

charris commented Sep 26, 2021

risicle commented Mar 6, 2022 • edited Loading

melissawm commented Mar 7, 2022

pearu commented Mar 8, 2022

risicle commented Mar 8, 2022

risicle commented Mar 11, 2022

risicle commented Mar 11, 2022

pearu commented Aug 31, 2021 •

edited

Loading

risicle commented Mar 6, 2022 •

edited

Loading