Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a 'safe' or similar command-line flag #2782

Open
arporter opened this issue Nov 14, 2024 · 8 comments
Open

Add a 'safe' or similar command-line flag #2782

arporter opened this issue Nov 14, 2024 · 8 comments
Labels
enhancement good first issue An issue that is probably suitable for a new PSyclone developer

Comments

@arporter
Copy link
Member

Finding out where things are going wrong when using PSyclone to process existing code can be hard. It is proposed to add a --safe flag or similar akin to -O0 on a compiler. This will turn off things that we know can be problematic, e.g.:

  • Ensure no transformations are applied
  • Disable canonicalisation
    • e.g. put multi-statement WHERE constructs into CodeBlocks (or just all WHERE statements?)
    • CASE statements
  • Disable line-length limiting?

There are probably other things. Suggestions from @hiker, @TeranIvy, @sergisiso and @LonelyCat124 are welcome!

@arporter arporter added enhancement good first issue An issue that is probably suitable for a new PSyclone developer labels Nov 14, 2024
@arporter
Copy link
Member Author

I've (perhaps optimistically) given this the 'good-first issue' - at least adding the command-line flag and populating something in the Config object should be reasonably straightforward (as much as anything ever is).

@arporter
Copy link
Member Author

We may want to support selectively turning-off the various things I listed in the original description. Therefore the precise form of the command-line flag needs some thought.

@LonelyCat124
Copy link
Collaborator

I think --safe as a "turn on all of these options" is a good idea (with its help flag saying what it turns on, similar to how -O3 turns on e.g. -ffast-math and -floop-unrolling and etc. etc.) and having various other options as things that can be turned on/off one-at-a-time to check where the disparity comes from between --safe and a standard execution of PSyclone.

@sergisiso
Copy link
Collaborator

I understand the point of "safe" for the frontend (we do some cannonicalisations that can fail and have some ambiguities coming from fparser; and we have do way to disable/stop them), but I don't think we need to do:

  • Ensure no transformations are applied
  • Disable line-length limiting?

The can already do these by changing flags (no -s -> no transformations), and would will make the "safe" functionality very complex if it is more broad than the front-end.

@LonelyCat124
Copy link
Collaborator

I think if these are already flags (e.g. -s /dev/null and -l unlimited or something) having the psyclone entry point part of --safe just override any other values for these flags could be a straight-forward solution to enable those.

@sergisiso
Copy link
Collaborator

But if it overrides the values we can not try things like "safe frontend but with transformations enabled" or "safe frontend but with line length limits enabled" which can help identify exactly what produces the issues:

psyclone --safe -l off ...  # with no transformation and no line limits
psyclone --safe -l off -s transform.py ... # with transformation and no line limits
psyclone --safe -s transform.py ... # with transformation and line limits
psyclone -s transform.py ... # all enabled

@LonelyCat124
Copy link
Collaborator

Yeah, i think my idea was more

psyclone --safe .... # Fully safe
psyclone -l off -s transform ... # Transform enabled, WHERE statements transformed, no line limits
psyclone --nowhere -s transform # Transform enabled, WHERE enabled, line limits

so safe is just a superceded set of flags and once safe works you have a set of flags you can remove one at at time until things break. I do like your idea though as it makes it easier to re-enable bits individually (but requires users to correctly remove flags to use --safe). We could have --safe as full override and a separate flag to add any safe flags not overridden? But gets a bit complicated.

@arporter
Copy link
Member Author

arporter commented Nov 15, 2024

Could we have something like:

psyclone --safe=all
psyclone --safe=nowhere,no_stmt_fns

I take your point about the line-length limiting etc. Perhaps the rule should be that if an option can already be controlled by a flag then it should not be part of 'safe' (although we could print a warning if e.g. --safe is used in conjunction with -s)?

Perhaps we should draft a 'Troubleshooting' section of the User Guide. That will help us think this through.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement good first issue An issue that is probably suitable for a new PSyclone developer
Projects
None yet
Development

No branches or pull requests

3 participants