PSyclone icon indicating copy to clipboard operation
PSyclone copied to clipboard

Remove PSyData names from config file

Open hiker opened this issue 1 year ago • 3 comments

ATM the config file lists all valid psydata prefixes:

VALID_PSY_DATA_PREFIXES = profile, extract, read_only_verify, nan_test

Since now LFRic contains its own config file, it's hard to add new psy data prefixes, they require a change to LFRic. This happened by #2690, which renames nan_test to value_range_check - in order to use this with LFRic, I now also need to modify LFRic's psyclone.cfg file.

Options:

  1. We just remove this list and the check for valid prefixes in PSyclone. TBH, I doubt it does a lot to make things more correct, psydata is somewhat advanced anyway, so will be used (or setup to be used) by experienced programmer anyway.
  2. We add command line options to add more valid psydata prefixes.

I would prefer 1., but am happy for feedback.

hiker avatar Aug 22 '24 14:08 hiker

I vote for 1. But why do we have to check for valid prefixes at all and not just be fine with what the user says in the transformation option?

sergisiso avatar Aug 23 '24 08:08 sergisiso

I vote for 1. But why do we have to check for valid prefixes at all and not just be fine with what the user says in the transformation option?

I would need to go through PR logs, I think it was a result of the code review, with the intention to limit the amount of mixing different psy_data at the same time (i.e. it makes only sense to have ONE profiling lib at a time, but you could do the old nan-testing with readonly-not-overwritten-checks). So in order to make sure no one simple changes the prefix in a file, we added that test ... I think.

hiker avatar Aug 23 '24 13:08 hiker

If the reason is not clear I would prefer to remove the restriction altogether, sure that in general makes sense to do one profiling at a time, but I still think it is for the script writer (HPC expert) to decide.

sergisiso avatar Aug 23 '24 14:08 sergisiso