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

Started on: making Loki plan trafo (pipeline) aware #444

Closed
wants to merge 1 commit into from

Conversation

MichaelSt98
Copy link
Collaborator

Corresponding CLOUDSC branch: nams-cloudsc-loki-plan (necessary since CLOUDSC wasn't using loki_transform_target yet).

  • more or less just copied/utilised the infrastructure and mechanism for (dispatching) transformations and duplicated for planning
  • the only trafo that uses this for now is the FileWriteTransformation
    • consequently write_cmake_plan is not renaming the modules/routines anymore

Next step:

  • create new item (and/or delete existing item)
    • I've added cloudsc_init.F90 in the above CLOUDSC branch as well as a DuplicateKernel and RemoveKernel trafo to test that (obviously not working yet)

Copy link

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

Copy link
Collaborator

@reuterbal reuterbal left a comment

Choose a reason for hiding this comment

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

Many thanks for this @MichaelSt98 !

I looked through this yesterday and noticed the need for significant code duplication if we make the planning a separate traversal mode in the Scheduler. So, I've played around with an approach, where planning becomes an option in the Scheduler's process and Transformation's apply entry points, with the latter taking care of dispatching to plan_<...> instead of transform_<...>. For me, that has the advantage that the planning mode really becomes closely linked to the processing of pipelines/transformations.

However, I have not gone as far as testing your DuplicateKernel/RemoveKernel transformations or CLOUDSC in this setup.

You can take a look at the changes here:
main...nabr-pipeline-plan

Note that this sits on top of my changes from #441, because that allowed me to utilize the additional tests added there. I have rebased the branch following the merge, the diff is now clean.

FYI: @mlange05

@MichaelSt98
Copy link
Collaborator Author

Out-dated and superseded due to/by #453.

@MichaelSt98 MichaelSt98 closed this Dec 2, 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 this pull request may close these issues.

2 participants