Skip to content

313 unify flag handling#504

Open
hiker wants to merge 43 commits intomainfrom
313_unify_flag_handling
Open

313 unify flag handling#504
hiker wants to merge 43 commits intomainfrom
313_unify_flag_handling

Conversation

@hiker
Copy link
Collaborator

@hiker hiker commented Oct 10, 2025

This is getting reasonable stable, but it's currently blocked by #502 (which I need for kernel extraction in gungho). I will also verify that this works as expected with the UMm

@hiker hiker marked this pull request as draft October 10, 2025 13:06
@hiker hiker self-assigned this Oct 10, 2025
@hiker hiker added enhancement New feature or request Blocked Blocked by another issue labels Oct 10, 2025
@hiker hiker removed the Blocked Blocked by another issue label Oct 27, 2025
if isinstance(new_flags, str):
self.append(new_flags)

class AlwaysFlags(AbstractFlags):
@hiker hiker marked this pull request as ready for review January 28, 2026 05:18
@hiker hiker added this to the v2.0.2 - UM support milestone Jan 28, 2026
@hiker
Copy link
Collaborator Author

hiker commented Jan 28, 2026

This PR adds a set of classes to manage Flags. It allows one FlagList to be used everywhere, which can be a mixture of generic flags (that always apply) and path-specific flags.

I have NOT changed the API of Fab itself, most steps take two flag lists (common flags and path flags). If we agree to change the API all these two arguments can be replaced with a single FlagList.

Internally, the path flags are converted to the new MatchFlags, and all handling internally is with FlagList.

@yaswant , @MatthewHambley , @Pierre-siddall, @t00sa , @cameronbateman-mo
Ready for a first review.

I did see the automatic comment that I am not calling the super class - the super class is an abstract class that has no implementation of init, and if I add , I get flake8 errors (this is already commented and closed, but it just got raised again). Let me know what I should do there :)

@hiker hiker added the Ready for review Indicating that a PR is ready to be reviewed. label Jan 28, 2026
@hiker
Copy link
Collaborator Author

hiker commented Feb 9, 2026

@yaswant , @MatthewHambley , @Pierre-siddall, @t00sa , @cameronbateman-mo , this has still not been assigned.

Given the breaking lfric builds atm (ultimately because of a intel compiler bug which can't handle a specific OMP continuation line created by a new transmuation script), I have added a small change to also allow psyclone to have flags added (i.e. I made Psyclone a ToolWithFlags instead of a Tool). This way, a site can simple add any additional PSyclone options (like --backend-disable-indentation) if required.

One question:
ATM, if a ProfileFlag does not have a definition for a given profile (when flags for this profile are requested), it will raise an exception. I wonder if it would be more convenient to just return [] in this case. For example, PSyclone will typically have no profile, so I had to add a try/except clause when querying the flags. I am not sure about the best way:

  • strict (the way it is, a caller must make sure the profile mode is actually defined for a tool that uses a ProfileFlag), i.e. the way it is atm
  • somewhat relaxed (add an option to get_flags where a user can specify to ignore a missing profile or not
  • more relaxed (if a tool has no profile defined at all, just return [])
  • totally relaxed (always return [])? Risk here is that a typo in a profile-name (on the user site)

My feeling is that the maintainers here prefer to be as strict as possible, so that's what I implemented. I personally would prefer one of "somewhat relaxed" or "more relaxed" :)

@yaswant yaswant requested a review from t00sa February 10, 2026 11:52
@hiker hiker mentioned this pull request Feb 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request Ready for review Indicating that a PR is ready to be reviewed.

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant