stfc / PSyclone

Domain-specific compiler and code transformation system for Finite Difference/Volume/Element Earth-system models in Fortran
BSD 3-Clause "New" or "Revised" License
104 stars 28 forks source link

[PSyData] Handling LFRic constants in Kernel Extraction #1737

Open hiker opened 2 years ago

hiker commented 2 years ago

While trying to manually fix up an LFRic driver for a gungho run, I ran into the problem that the kernel used a function that used some constants and configuration info (Rd, p_zero, kappa, scaled_radius, coord_system).

This leads to three problems:

Especially the last point is a big concern. I've manually just removed the 'protected' attribute so that I could initialise these constants, but that's obviously not a good solution. The values are either pre-computed or read from a config file - and we don't want to go through the whole lfric initialisation (not to mention that this would mean we need to support namelist files for our driver code to run).

Best idea: maybe we can add the functions that are being called to the driver file - this would mean we don't need to link with any lfric files (??? not sure, some infrastructure files might still be required???). And then we could either change where these constants come from, or just replace the names with the numerical values??

arporter commented 2 years ago

I've been hitting similar issues in trying to make an executable test harness for adjoint kernels. Basically the LFRic infrastructure has moved on a lot from our chopped version and I'm unable to create code that compiles for both - in particular the function-space-type and mesh-type constructor arguments have changed. It seems that the 'proper' way to do it is to use collections for both of these things but I couldn't get that to work so have given up for now. We need to agree a way forward.

arporter commented 2 years ago

(The approach I've taken to getting a working test harness is to generate code that must then be plugged into a 'proper' LFRic mini-app - this then provides things like chi and panel_id.)

hiker commented 2 years ago

I actually linked with all (or nearly all) gungho .o files (I tried to be selective, but it's just a dependency nightmare). I still can run my driver test without anything else, since no lfric related code is ever called (i.e. I need to link with xios, yaxt, mpi, ...). And yes, our infrastructure is badly outdated, doesn't even compile with LFRic anymore (#1735 might make it a bit easier to just point to a different infrastructure library, that at least works for psydata related stuff).

It took me a LONG time to manually fix up my driver, since I manually had to change nearly all type declarations (and warning: at least the gnu compiler does NOT complain if you pass a 2d-array to a kernel that expects a 4d-array, if this 4d array is declared with explicit bounds, not assumed-shape ... I didn't know that 😢 ), but except for these constants (which for now only work because I removed the protected attribute in the modules), it seems to work just fine (Ok, result differences after 6 digits).

Is your test harness based on an actual kernel call? I could try to run the kernel data extraction on that kernel (which should give you chi and panel_id and everything else you need for the actual call), and then fix up the driver - I think now it would be a lot faster for me to do that (have opened a ticket or two, and already hacked improvements in place). Only disadvantage is that you need to have a XX MB netcdf file (in my case 11MB). Let me know if you are interested.

hiker commented 2 years ago

I was successful to remove all of the LFRic infrastructure/files from my test case (by copying the required kernels and functions into the driver file, and then replacing the constants imported from modules with a new dummy module that provides the constants). But I had to remove several unused functions from modules (since they triggered more dependencies,w which then required a significant part of the infrastructure files :( ), and it also required #1741.

All in all, I think it would be extremely hard to fully automate the process of creating really stand-alone driver, so we might need to re-adjust our goal realistically. Linking with the infrastructure appears to make this task much easier (though it still leaves the originally raised issues about exporting/importing and initiaising constants)

hiker commented 1 year ago

This is mostly a duplicate of #1990, but it mentions the problem of protected attributes :(

Potential solution could be to require explicit setter functions to be implemented (e.g. for member XYZ there would have to be set_XYZ - but that seems to be hard to enforce (since it's outside of our control). I would hope that instead we can use the PSyIR to modify all private to public when inlining (atm in #1991 we inline by just copying the text files, but additional functionality is already being created that would make this easy to implement, except the few files that cannot be converted to psyir due to pre-processor directives).