1623 add training #3145

hiker · 2025-09-21T13:21:44Z

Adds the PSyclone training material (based on #3131)

… now inlines.

…i atm).

…lass

…_training

…amples fully works.

…ng pandoc.

hiker · 2026-01-06T06:28:47Z

@arporter , I have addressed all changes. I also added a lot of additional tests, since I had at least two examples being broken that were not picked up by my testing - which was pretty bad in the actual training sessions. One issue is fixed already (and also properly tested here), one is still outstanding (#3252).

The fact that this tutorial contains additional important testing makes me push for getting this in sooner than later. Otherwise we risk a never-ending PR: as PSyclone gets updated and the training is not tested, there is a chance that something breaks the training, Then I need to debug and fix this (or get help to fix this), instead of these problems being flagged when the PSyclone improvements are added (and then being fixed by the original developer before the PR is merged).

Note that I have opened #3270 for additional training improvements (e.g. linking files instead of duplicating ... at least evaluating this option; adding syntax highlighting). And also #3208 to review the individual training sections.

While I totally agree that this is not ideal, my suggestion would be that the review here focuses just on the testing side-of things, to get any issues there sorted out. No one has the time to proof-read all of this text material, and as mentioned, delays this PR just mean this will keep on being more work to keep this up-to-date. Yes, there is a risk that it might take a long time before this material (as in: the READMEs) gets properly updated, and I understand your reluctance.

I am open for any suggestion you have, I don't have a particular brilliant idea either :(

Am triggering the full CI as well.

arporter

I'm enjoying going through (in outline) your game-of-life example - it's great to see that you can recover performance :-)
Since most of the example transformation scripts need their module and apply-method docstrings updating, I'm going to pause here to let you do that.
As you suggested, I'm basically ignoring the actual text of the readme's so I am by no means reviewing the actual tutorials.

arporter · 2026-01-06T08:14:34Z

tutorial/training/Makefile

+# Author: J. Henrichs, Bureau of Meteorology
+
+# The transform target `test` runs tests on the gitlab CI (i.e. without
+# compilation), `test_run` will compile all binaries (as far as possible


Do you mean compile instead of test_run and then s/run/test_run/ on the following line?

I've cleaned this up (including removing the 'compile' target from the top level makefiles ... most 'local' Makefiles have a compile for the convenience when running the training, but the high level ones should only use test or test_run)

arporter · 2026-01-06T08:16:41Z

tutorial/training/Makefile

+.PHONY: allclean clean compile test test_run
+
+allclean clean compile test test_run:
+	$(MAKE) -C psyclone_for_lfric_users $@


Ah, so this part is for LFRic users rather than developers? I think "lfric_users" would be fine since this is already under the "training" directory in the PSyclone repository.

If I do:

cd PSyclone/tutorial/training make compile

then I get:

make -C psyclone_for_lfric_users compile make[1]: Entering directory '/home/me/Projects/PSyclone/tutorial/training/psyclone_for_lfric_users' make[1]: *** No rule to make target 'compile'. Stop.

That was a left-over target, the top level Makefiles should only support 'test' and 'test_run' (many 'lowest level' makefiles support compile, since it's convenient for the user if e.g. they want to do performance measurements), but no need to support this everywhere.

arporter · 2026-01-06T08:19:03Z

tutorial/training/README.rst

+This is the training material for the usage of PSyclone, a code
+transformation and generator tool. PSyclone was originally developed
+for the UK Met Office’s new LFRic numerical weather prediction system,
+but it’s features of modifying large codes based on scripts have found


arporter · 2026-01-06T08:21:28Z

tutorial/training/README.rst

@@ -0,0 +1,90 @@
+PSyclone Training


Thanks for adding this Joerg - it's very helpful. Please could you also add a brief README in the directory above to explain what is contained in each of the subdirectories - i.e. it's not clear what the difference between 'training' and 'tutorial' is. Ideally, at some point, we'll probably want to merge the two.

arporter · 2026-01-06T08:21:44Z

tutorial/training/README.rst

+more open ended, and trainees are encouraged to go back to these
+examples and implement better solutions later.
+
+The trainings material is split into four parts, and the files


arporter · 2026-01-06T12:30:53Z

tutorial/training/gocean/2.8-GameOfLife-openmp/fuse_loops.py

+# -----------------------------------------------------------------------------
+# Author: J. Henrichs, Bureau of Meteorology
+
+'''Python script intended to be passed to PSyclone's generate()


Pls update.

arporter · 2026-01-06T12:31:09Z

tutorial/training/gocean/2.8-GameOfLife-openmp/fuse_loops.py

+
+def trans(psyir):
+    '''
+    Take the supplied psyir object, and fuse all loops.


Also inlines...

arporter · 2026-01-06T12:31:33Z

tutorial/training/gocean/2.8-GameOfLife-openmp/fuse_loops_last.py

+
+def trans(psyir):
+    '''
+    Take the supplied psyir object, and fuse the last three loops.


Also inlines.

arporter · 2026-01-06T12:31:48Z

tutorial/training/gocean/2.8-GameOfLife-openmp/openmp.py

+# -----------------------------------------------------------------------------
+# Author: J. Henrichs, Bureau of Meteorology
+
+'''Python script intended to be passed to PSyclone's generate()


Pls update.

arporter · 2026-01-06T12:34:36Z

tutorial/training/gocean/2.4-GameOfLife-psyclone/solution/read_config_mod.f90

+
+    line_count = 0
+
+    do


Comment please.

…ith a symlink.

hiker · 2026-01-08T04:55:34Z

Wow, that was an impressive review. I think it took me over an hour just to add 'done' to your comments. You rightly pointed out some repeated errors (generat function, missing docstrings, inlining not mentioned). I think I've fixed them up everywhere.

I've triggered the full IT as well.

hiker · 2026-01-08T06:19:26Z

@arporter I have a problem - turns out the OpenMP offloading does not work (anymore??). Due to a bug in the makefile, it actually hasn't applied the offloading script at all in my recent tests (I am pretty certain it did originally, but I did some refactoring/ simplifications of the Makefile, which I believe introduced this bug recently). Now I've fixed the Makefile, it aborts with:

make -C 2.16-GameOfLife-omp-offload/solution test_run
...
make --no-print-directory  run | tail -n 12 | diff -b - /archive/psyclone-tests/action-runner-software/actions-runner/_work/PSyclone-mirror/PSyclone-mirror/tutorial/training/gocean/gol-lib/glider.correct

libgomp: pointer target not mapped for attach

(see https://github.com/stfc/PSyclone-mirror/actions/runs/20805411529/job/59758702708)

Googling indicates that there might be a mapping missing (which we don't support afaik), or additional compiler options(?). I don't know what to do. I don't have (easy) access to hardware that would support managed memory, and I am not even certain what the right solution would be in this case :( Can you (or Sergi?) help? I assume you can log onto the machine and try running it there?

hiker added 30 commits February 21, 2024 11:44

#1623 Fixed pylint warning (the rest is unavoidable).

27194b4

#1623 Fixed templates, added new script, updated README.

19bee75

#1623 More updates to handson.

e8deeec

#1623 Display schedule before the error happens.

0a8df3b

#1623 Made comparisons in README clearer.

533fcbc

1623 Remove example 7, which is too difficult.

b416af9

Merge remote-tracking branch 'origin/master' into 1623_add_training

fe5dcd8

#2049 Moved write_driver into base class.

81fd81c

#2049 Moved get_driver_as_string to base class, meaning gocean driver…

287419f

… now inlines.

#2049 Add a compilation test of the gocean driver (works only with mp…

b64217a

…i atm).

#2049 Updated gocean extract example to work with inlining.

cb863c6

#2049 Moved flatten_signature to base class.

bbdba64

#2049 Moved some tests.

be81aec

#2049 Moved import_modules into base class.

2ea3ae9

#2049 Start to move add_all_kernel_symbols to the base class.

5df947d

#2049 Fixed tests and error in logic.

488ae3d

#2049 Moved rest of add_all_kernel_symbols to base class.

92722a6

#2049 Import go_wp for gocean driver.

44d4d2b

Merge branch '1992_mpi_support_for_extraction' into 2049_driver_basec…

ff148a0

…lass

#2049 Fix failing test.

23696d9

Merge branch '1992_mpi_support_for_extraction' into 2049_driver_basec…

767425d

…lass

Merge branch '1992_mpi_support_for_extraction' into 2049_driver_basec…

304a570

…lass

Merge branch '1992_mpi_support_for_extraction' into 2049_driver_basec…

b376483

…lass

Merge branch '1992_mpi_support_for_extraction' into 2049_driver_basec…

6711833

…lass

Merge remote-tracking branch 'origin/master' into 2049_driver_baseclass

fdc038c

Merge remote-tracking branch 'origin/master' into 2049_driver_baseclass

755d1da

Merge remote-tracking branch 'origin/1623_add_training' into 1623_add…

0147782

…_training

Merge branch '257_verify_loop_fusion' into 1623_add_training

08fdf88

Merge branch '2690_value_verification' into 1623_add_training

d8c1e97

#1623 Started to support new value_range_check library.

64456e8

#1623 Try to fix more test, esp. running with different compilers.

0470854

hiker temporarily deployed to integration January 5, 2026 08:47 — with GitHub Actions Inactive

#1623 Use mpif90 for compilation tests in the tutorial, so the MPI ex…

e59a6de

…amples fully works.

hiker temporarily deployed to integration January 5, 2026 09:31 — with GitHub Actions Inactive

hiker added 5 commits January 6, 2026 13:52

#1623 Added syntax highlighting, which helps to convert md to pdf usi…

6091df5

…ng pandoc.

#1623 Added high-level READMEs.

c4da21d

#1623 Removed debug output.

31a96dd

#1623 Added missing newline.

861a713

#1623 Addressed issues raised in review.

11752c4

hiker temporarily deployed to integration January 6, 2026 06:29 — with GitHub Actions Inactive

arporter added under review and removed reviewed with actions labels Jan 6, 2026

arporter requested changes Jan 6, 2026

View reviewed changes

arporter added reviewed with actions and removed under review labels Jan 6, 2026

hiker added 11 commits January 7, 2026 14:37

#1623 Removed two unnecessary files, and replaced the remaining one w…

b380760

…ith a symlink.

#1623 Started to address reviewers comments.

108fb9a

Merge remote-tracking branch 'origin/master' into 1623_add_training

6b4ce26

#1623 Fix flake8.

8c43ef2

#1623 Addressed more issues raised in review.

c809814

#1623 Added missing docstrings.

4c7c825

#1623 More updated based on code review.

724f873

#1623 Added more doc strings to Fortran files.

3f0438d

#1623 Removed unused compile target from top level Makefiles.

f6a10fe

#1623 Updated README with additional information.

d4735c8

#1623 Renamed psyclone_for_lfric_users to lfric_users.

fa501df

hiker deployed to integration January 8, 2026 04:21 — with GitHub Actions Active

1623 add training #3145

Are you sure you want to change the base?

1623 add training #3145

Conversation

hiker commented Sep 21, 2025

Uh oh!

hiker commented Jan 6, 2026

Uh oh!

arporter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hiker Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hiker commented Jan 8, 2026

Uh oh!

hiker commented Jan 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hiker Jan 8, 2026 •

edited

Loading