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

448 make public example #450

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

448 make public example #450

wants to merge 10 commits into from

Conversation

hiker
Copy link
Collaborator

@hiker hiker commented Jul 18, 2024

Adds a script that removes all private and protected attributes (which is used in PSyclone's kernel extraction).
Added a chapter explaining the examples to the manual, added tests for examples (triggered in CI), and added me as author.

Copy link

codecov bot commented Jul 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (master@2d8cef7). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #450   +/-   ##
=========================================
  Coverage          ?   92.13%           
=========================================
  Files             ?       85           
  Lines             ?    13824           
  Branches          ?        0           
=========================================
  Hits              ?    12737           
  Misses            ?     1087           
  Partials          ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@arporter arporter left a comment

Choose a reason for hiding this comment

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

Looks good Joerg, thanks for adding to the docs and adding tests. Just a bit of tidying required.


create_dependencies.py
^^^^^^^^^^^^^^^^^^^^^^
This file analysis the dependencies between a set of Fortran files, based on
Copy link
Member

Choose a reason for hiding this comment

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

s/analysis/analyses/

that will read this file, execute the kernel, and compare the results with
the original results.

Since PSyclone will follow call tree, the code must be able to read even
Copy link
Member

Choose a reason for hiding this comment

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

"...follow the call tree,..."

run time of the script, it will use these values as default values in the
makefile. But by setting these environment variables when running ``make``,
these defaults can always be overwritten. The Makefile also has a ``clean``
target, which will remove all ``.mod``, object, and the program file (if
Copy link
Member

Choose a reason for hiding this comment

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

Good stuff, thanks for adding this Joerg.

@@ -33,17 +33,22 @@
# ------------------------------------------------------------------------------
# Author: J. Henrichs, Bureau of Meteorology

Copy link
Member

Choose a reason for hiding this comment

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

Comment please, e.g. "Makefile driver to test the various examples"

# -----------------------------------------------------------------------------
# BSD 3-Clause License
#
# Copyright (c) 2020, Science and Technology Facilities Council.
Copy link
Member

Choose a reason for hiding this comment

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

Oh my life, this has been a while! Apologies. Could update to 2020-24 now?

# ------------------------------------------------------------------------------
# Author: Joerg Henrichs, Bureau of Meteorology

"""This file contains an fparser script that parses Fortran files
Copy link
Member

Choose a reason for hiding this comment

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

This docstring needs changing (and possibly explains the 2020 date ;-) ).

all_nodes = list(walk(parse_tree, Attr_Spec))
for node in all_nodes:
if str(node) == "PROTECTED":
# This is a tuple, so we can't simple remove the attribute
Copy link
Member

Choose a reason for hiding this comment

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

"simply"

for node in all_nodes:
if str(node) == "PROTECTED":
# This is a tuple, so we can't simple remove the attribute
node.parent.items = tuple(i for i in node.parent.items if i is not node)
Copy link
Member

Choose a reason for hiding this comment

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

That's neat!

@arporter arporter added reviewed with actions PR has been reviewed and is back with developer and removed under review labels Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reviewed with actions PR has been reviewed and is back with developer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants