BUG: f2py incorrectly translates dimension declarations. #17654
Conversation
In fortran functions passed to f2py, calculations that occur in the dimensions of array declarations are interpreted as floats. Valid fortran requires integers. Problem only occurs with fortran functions not subroutines as only for the former does f2py generate fortran wrapper code that fails to compile. Relevant code is in numpy.f2py.crackfortran.getlincoef(), which calculates and returns the coefficients to getarrlen() for writing to the fortran wrapper code. Fixes numpygh-8062.
|
Hello, @ianthomas23 ! Thanks for this. Note that this only happens if the expression inside the dimension declaration is actually simplified by f2py (which is not the case for a non-linear expression like I think the Shippable failure is unrelated - don't understand what happened there. |
|
s390x failure not connected to this PR |
|
Thanks @ianthomas23 |
| except: | ||
| pass | ||
| try: | ||
| b = int(b) | ||
| except: | ||
| pass |
There was a problem hiding this comment.
Bare except is an antipattern, as this means that f2py now not only swallows Ctrl+C but give different compilation results depending on whether it was pressed. This should be except ValueError or at least except Exception?
Would you mind making a follow-up PR?
There was a problem hiding this comment.
Sorry, that was my oversight. I can do a follow up if @ianthomas23 prefers.
There was a problem hiding this comment.
For reasons I don't understand, LGTM caught this, but decided to give a CI tick anyway.
There was a problem hiding this comment.
@jhelie, I think we might have discussed this before - have we forgotten a config option, or is this working as intended?
There was a problem hiding this comment.
@eric-wieser, @melissawm: My apologies, I will submit a follow-up PR to change except to except ValueError. I'll wait until gh-17662 is accepted to avoid having to rebase.
There was a problem hiding this comment.
You shouldn't need to rebase on that PR, since the lines aren't next to each other - github will sort out the merge automatically I'd expect
There was a problem hiding this comment.
Say, aren't both try / except clauses redundant here?
The original values of a and b are created by myeval(), which raises an exception if the return type is not a float or int:
numpy/numpy/f2py/crackfortran.py
Lines 2105 to 2109 in ab22e00
| code = """ | ||
| function test(n, a) | ||
| integer, intent(in) :: n | ||
| real(8), intent(out) :: a(0:2*n/2) |
There was a problem hiding this comment.
What shape should a(0:n/2*2) give when n is odd?
There was a problem hiding this comment.
Good question. Fortran uses integer maths is array declarations, so that shape here, in Python syntax, is (1+2*(n//2),)
I have submitted a new PR (gh-17687) to fix the original problem and it includes a test of this example.
There was a problem hiding this comment.
What does -(-n)/2*2 give in fortran? (edited) The python integer division is not the same as the integer division used in most languages.
Corrections to PR numpygh-17654 following post-merge review. Fix now only casts floats to integers if they are definitely integers. Also, division that occurs in fortran array declarations is evaluated in an integer context, so f2py.crackfortran.myeval does the same, converting single to double slashes. Fixed numpygh-8062.
REV: Revert gh-17654 - f2py incorrectly translates dimension declarations.
In fortran functions passed to f2py, calculations that occur in the dimensions of array declarations are interpreted as floats. Valid
fortran requires integers. Problem only occurs with fortran functions not subroutines as only for the former does f2py generate fortran wrapper code that fails to compile.
Relevant code is in
numpy.f2py.crackfortran.getlincoef(), which calculates and returns the coefficients togetarrlen()for writing to the fortran wrapper code.To convert floats to ints I have opted for
try..exceptblocks which are common incrackfortran.pyrather than explicitis_int()checks and conversion usingint().Fixes gh-8062.