Closed TeranIvy closed 6 years ago
Hi Iva, thanks for opening this issue. Would you like to take this one on?
Hi Andy, I am happy to. It is a great opportunity to dig into fparser! The only problem is that I cannot assign myself. Do I need special permissions?
Great! Do of course feel free to ask for help (although we too are slowly learning about how it actually works).
Hi Iva, many thanks for taking this on. I thought I would just mention that while we are currently using fparser in PSyclone and fixing any bugs/adding new features, such as this one, our long term plan is to move to use fparser2, which is also in this repository. I'm just mentioning it in case the two implementations cause confusion.
Hi @rupertford, does this mean that a fix for this issue needs to be applied in fparser2 as well?
Hi Iva, it is probably not supported in fparser2 but you don't need to worry about that in this issue.
Hi @arporter and @rupertford! I think I found the bug so I checked out a new branch and corrected it. Unfortunately I cannot push the commit as I do not have the relevant permission:
ERROR: Permission to stfc/fparser.git denied to TeranIvy.
Can you please add me? Also, I presume that the procedure of testing (and the required Python packages and paths) are similar to PSyclone? Thanks!
UPDATE: I just accepted Andy's invitation to collaborate, thanks!
Good work Iva. Yes, we apply the same coding rules (test coverage, pep8, pylint, style etc.), see https://github.com/stfc/fparser/wiki/CodeReview.
In fact, that wiki page is now out of date as we've adopted an additional rule: the interface to any new/modified routines must be fully documented using Sphinx markup (Rupert loves this ;-)). e.g.:
def parse(alg_filename, api="", invoke_name="invoke", inf_name="inf",
kernel_path="", line_length=False,
distributed_memory=config.DISTRIBUTED_MEMORY):
'''Takes a GungHo algorithm specification as input and outputs an AST of
this specification and an object containing information about the
invocation calls in the algorithm specification and any associated kernel
implementations.
:param str alg_filename: The file containing the algorithm specification.
:param str invoke_name: The expected name of the invocation calls in the
algorithm specification
:param str inf_name: The expected module name of any required
infrastructure routines.
:param str kernel_path: The path to search for kernel source files (if
different from the location of the algorithm
source).
:param line_length: A logical flag specifying whether we care about...
:type line: bool
:rtype: ast,invoke_info
:raises IOError: if the filename or search path does not exist
:raises ParseError: if there is an error in the parsing
'''
I'll update the wiki now...
Test on LFRic trunk works - it generates the correct statement (USE, INTRINSIC :: ieee_arithmetic
) and compiles and runs normally.
fparser2 source and test files (Fortran2003.py and _testFortran2003.py) seem to generate the correct code for this case.
Hi @TeranIvy, you only need to make lines that you touch pep8 clean. If this results in 'reduced' test coverage then that's fine - it simply means the test coverage figure is now more accurate!
There are currently two algorithms in the LFRic trunk which cannot be compiled due to fparser incorrectly parsing
use, intrinsic :: ieee_arithmetic
asUSE INTRINSIC :: ieee_arithmetic
(it should be left as is). As it is planned to useieee_arithmetic
module in all LFRic solvers (LFRic ticket #1222), this needs to be corrected.