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

POC: allow multi-level/subgraph duplication of kernels #474

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MichaelSt98
Copy link
Collaborator

POC duplicating a kernel and optionally duplicating/separating the underlying subgraph.

E.g., starting with:

callgraph_orig.pdf

the result of the DuplicateKernel trafo with duplicate_kernels=('kernel',) is:

callgraph.pdf

However, with this PR one can specify duplicate_subgraph=True, which yields for the example above:

callgraph.pdf

Further, the actual transformation part works as well, producing e.g.,:

MODULE kernel_mod_duplicated
  IMPLICIT NONE
  CONTAINS
  SUBROUTINE kernel_duplicated (klon, field1)
    USE kernel_nested_mod_duplicated, ONLY: kernel_nested_vector_duplicated, kernel_nested_seq_duplicated
    USE compute_2_mod_duplicated, ONLY: compute_2_duplicated
    IMPLICIT NONE
    INTEGER, INTENT(IN) :: klon
    INTEGER, INTENT(INOUT) :: field1(klon)
    INTEGER :: tmp1(klon)
    INTEGER :: jl
    
    CALL kernel_nested_vector_duplicated(klon, field1)
    
    DO jl=1,klon
      CALL kernel_nested_seq_duplicated(field1(jl))
      CALL compute_2_duplicated(field1(jl))
      tmp1(jl) = 0
      field1(jl) = tmp1(jl)
    END DO
    
  END SUBROUTINE kernel_duplicated
END MODULE kernel_mod_duplicated

@MichaelSt98 MichaelSt98 requested a review from reuterbal January 15, 2025 14:39
Copy link

Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/474/index.html

Copy link

codecov bot commented Jan 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.05%. Comparing base (c054b46) to head (7dff96e).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #474      +/-   ##
==========================================
+ Coverage   96.03%   96.05%   +0.01%     
==========================================
  Files         226      226              
  Lines       40579    40639      +60     
==========================================
+ Hits        38970    39035      +65     
+ Misses       1609     1604       -5     
Flag Coverage Δ
lint_rules 96.39% <ø> (ø)
loki 96.04% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@@ -545,6 +545,8 @@ def _get_definition_items(_item, sgraph_items):
if transformation.renames_items or transformation.creates_items:
kwargs['item_factory'] = self.item_factory
kwargs['scheduler_config'] = self.config
kwargs['graph'] = graph
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to consider whether we want to directly provide the SGraph to transformations or build an API to semantically capture the changes and have the scheduler apply them afterwards.
The latter seems more elegant (not hacking with core data structures) but it might be tricky to build something equally flexible.
Also note that tthe graph may be a temporary object here when traversing a file graph. In that case, this would not work at all.

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 this pull request may close these issues.

2 participants