Skip to content
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.

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

Definitions not attached if multiple scopes defined in the same Sourcefile #161

Closed
rommelDB opened this issue Oct 2, 2023 · 1 comment · Fixed by #325
Closed

Definitions not attached if multiple scopes defined in the same Sourcefile #161

rommelDB opened this issue Oct 2, 2023 · 1 comment · Fixed by #325

Comments

@rommelDB
Copy link
Contributor

rommelDB commented Oct 2, 2023

Regarding the execution of the following code:

from loki import Sourcefile, Subroutine, FindVariables
from pprint import pprint

fcode = """
module foo
  implicit none
  integer, parameter :: j = 16

  contains
    integer function SUM(v)
      implicit none
      integer, intent(in) :: v
      SUM = v + 1
    end function SUM
end module foo

module test
  use foo
  implicit none
  integer, parameter :: rk = selected_real_kind(12)
  integer, parameter :: ik = selected_int_kind(9)

  contains
    subroutine calc (n, res)
      integer, intent(in) :: n
      real(kind=rk), intent(inout) :: res
      real(kind=rk), dimension(n) :: vec

      integer(kind=ik) :: i

      do i = 1, n
         vec(i) = 1.0_rk/(real(n - i + 1, kind=rk))
      end do

      do i = 1, n
        res = res + SQRT(real(i, kind=rk)) + SUM(j)
      end do
    end subroutine calc

end module test
"""

source = Sourcefile.from_source(fcode)
calc_subroutine = source['calc']
vars = FindVariables().visit(calc_subroutine.body)
pprint(vars)

Actual output:

{Array('vec', None, None, None, (Scalar('i', None, None, None),)),
 DeferredTypeSymbol('REAL', None, None, <SymbolAttributes BasicType.DEFERRED>),
 DeferredTypeSymbol('SQRT', None, None, <SymbolAttributes BasicType.DEFERRED>),
 DeferredTypeSymbol('SUM', None, None, <SymbolAttributes BasicType.DEFERRED>),
 DeferredTypeSymbol('j', None, None, <SymbolAttributes BasicType.DEFERRED>), 
 Scalar('i', None, None, None),
 Scalar('rk', None, None, None),
 Scalar('n', None, None, None),
 Scalar('res', None, None, None)}

I wanted to print variables only, however, I didn't expect to REAL, SQRT, and SUM be in the list.

I'm aware that it is the current definition, though:

retriever = ExpressionRetriever(lambda e: isinstance(e, (Scalar, Array, DeferredTypeSymbol)))

So, in order to obtain the true type of deferred symbol. Would a new pass or something like that be required? I would appreciate any feedback.

@reuterbal
Copy link
Collaborator

Hi Rommel,
very good question and let me try to provide a few pointers why we don't resolve these symbols currently:

  1. Let me start by explaining why DeferredTypeSymbol is included in the list of variables: This is because we represent that way any symbols for which we cannot conclude what type they have. That includes, among others, symbols that are imported from other scopes, e.g., module variables.
    Importantly, when definitions are available they are automatically attached, but your example points out an interesting limitation where this doesn't happen because multiple "top-level" scopes are defined within the same source file. Putting the modules into separate Sourcefile objects (which is what we typically find in our code base) would be a workaround how SUM (which overwrites the definition of a Fortran intrinsic) will be correctly recognized as a ProcedureSymbol, which we don't include in the variables lookup:
from loki import Sourcefile, Subroutine, FindVariables
from pprint import pprint

fcode_foo = """
module foo
  implicit none
  integer, parameter :: j = 16

  contains
    integer function SUM(v)
      implicit none
      integer, intent(in) :: v
      SUM = v + 1
    end function SUM
end module foo
"""

fcode_test = """
module test
  use foo
  implicit none
  integer, parameter :: rk = selected_real_kind(12)
  integer, parameter :: ik = selected_int_kind(9)

  contains
    subroutine calc (n, res)
      integer, intent(in) :: n
      real(kind=rk), intent(inout) :: res
      real(kind=rk), dimension(n) :: vec

      integer(kind=ik) :: i

      do i = 1, n
         vec(i) = 1.0_rk/(real(n - i + 1, kind=rk))
      end do

      do i = 1, n
        res = res + SQRT(real(i, kind=rk)) + SUM(j)
      end do
    end subroutine calc

end module test
"""

source_foo = Sourcefile.from_source(fcode_foo)
source_test = Sourcefile.from_source(fcode_test, definitions=source_foo.definitions)
calc_subroutine = source_test['calc']
vars = FindVariables().visit(calc_subroutine.body)
pprint(vars)
{Array('vec', None, None, None, (Scalar('i', None, None, None),)),
 DeferredTypeSymbol('REAL', None, None, <SymbolAttributes BasicType.DEFERRED>),
 DeferredTypeSymbol('SQRT', None, None, <SymbolAttributes BasicType.DEFERRED>),
 Scalar('res', None, None, None),
 Scalar('n', None, None, None),
 Scalar('i', None, None, None),
 Scalar('rk', None, None, None),
 Scalar('j', None, None, None)}

I consider this a bug but one that should be fairly easy to fix, if you wanted to try that. I suspect adding a module to definitions when it is instantiated might be one way of achieving that, e.g., here for fparser:

return Module(name=name, parent=kwargs['scope'])

  1. Achieving the same for the other intrinsic functions REAL and SQRT is a little trickier, as it requires baking in the information about Fortran intrinsics, which the frontends have to provide to us. Moreover, it would likely require introducing dedicated scopes for each of the intrinsic modules. In the scope attacher we are already using that information from Fparser:

    from fparser.two.Fortran2003 import Intrinsic_Name
    _intrinsic_fortran_names = Intrinsic_Name.function_names

    to gracefully step over intrinsic symbols:
    elif expr not in _intrinsic_fortran_names:
    debug('AttachScopesMapper: %s was not found in any scopes', str(expr))

    Unfortunately, this does not include information about the types of arguments and, more importantly, the return type of these functions:
    https://github.com/stfc/fparser/blob/f43a18fdb9b03a70fa0f5b9a04e3e01b12533360/src/fparser/two/Fortran2003.py#L12027
    Therefore, even then we wouldn't be able to deduce the full type of these symbols. Ideally, this information should therefore be added to fparser first, and then picked up by Loki to fully resolve the type of these symbols.
    A short-term workaround for your use-case would of course be to just filter the list of variables by name using the intrinsic names property, which by itself might be a useful utility routine already.

  2. Lastly, once the full type information was available we would even be capable to implement checks for correct type in expressions, which would be a very useful Linter rule, but is dependent on the above points to be resolved first, of course.

I hope that helps and feel free to come back with follow-up questions of course!

@reuterbal reuterbal changed the title Question about DeferredTypeSymbol Definitions not attached if multiple scopes defined in the same Sourcefile Oct 4, 2023
reuterbal added a commit that referenced this issue Jun 10, 2024
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 a pull request may close this issue.

2 participants