-
Notifications
You must be signed in to change notification settings - Fork 32
Description
Since the change to gocen to use the arrays instead of fields (i.e. field%data instead of field), there need for domain-specific implementations of PSydata has gone, but as a side effect, the value range check functionality is broken:
Since the Fortran range check function received field%data as string, it checks for environment variables called PSYVERIFY__module__kernel__field%data, instead of looking for PSYVERIFY__module__kernel__field.
And of course % is not a valid part of an environment variable :(
Not sure how to best fix this. Immediate idea: in the Fortran psydata libraries remove everything after the first % from the variable name. This would work for gocean, but in generic Fortran, a user might have variables a%b and a%c, and might want to specify different valid ranges for both of them, which then would not be possible. In gocean we also do get variables like neighbours%internal%xstart, i.e. multiple % (though I would accept that loop boundaries should not need to be supported?)
Or we replace any % with _. But then the user would have to specify field_data to specify the range for field, which is not entirely intuitive, the user should not need to know that there is a data anywhere :(
Suggestions welcome.
Second issue: this bug should have been detected by the CI. We do have a test, but what obviously happened was that the test was updated to expect field%data. We also have examples/gocean/eg5, but that only tests for infinity, not that setting a variable work. Similarly, the training did not actually test this at all (so that will become part of #1623).
Summary:
- Find and implement (likely in the Fortran wrapper and the transformations?) the best way to support gocean.
- Add/Update tests or examples to check that setting environment variables works
- Add/update training to test this properly (this will be done as part of Add training material to PSyclone #1623, just adding this here to remind me :) )