Skip to content

Use proper prefix when checking options_left - #44

Open
JHopeCollins wants to merge 3 commits into
mainfrom
JHopeCollins/bugfix/scope-options-left
Open

Use proper prefix when checking options_left#44
JHopeCollins wants to merge 3 commits into
mainfrom
JHopeCollins/bugfix/scope-options-left

Conversation

@JHopeCollins

Copy link
Copy Markdown
Member

Fixes using -prefix_options_left for scoped warnings.

@JHopeCollins
JHopeCollins requested a review from connorjward July 21, 2026 18:32
@JHopeCollins JHopeCollins self-assigned this Jul 21, 2026
Comment thread petsctools/options.py Outdated
@connorjward

Copy link
Copy Markdown
Collaborator

I didn't realise that this could be a non top level option. What happens if you have:

-options_left 1
-prefix0_options_left 0
-prefix0_prefix1_options_left 1

@JHopeCollins

Copy link
Copy Markdown
Member Author

I didn't realise that this could be a non top level option. What happens if you have:

-options_left 1
-prefix0_options_left 0
-prefix0_prefix1_options_left 1

Good point, I didn't think of that case.
I read the global option (-options_left) first, and then read the scoped option (-prefix_options_left) second, so the more specific option will take precedence.
Usually I think that would be the Right Thing (e.g setting default values for -fieldsplit_option), but seeing as this is a purely debugging option then arguably you want to be able to set it globally.

I am erring on the side of keeping the current behaviour to be consistent with other options. What do you think?

@connorjward

Copy link
Copy Markdown
Collaborator

I didn't realise that this could be a non top level option. What happens if you have:

-options_left 1
-prefix0_options_left 0
-prefix0_prefix1_options_left 1

Good point, I didn't think of that case. I read the global option (-options_left) first, and then read the scoped option (-prefix_options_left) second, so the more specific option will take precedence. Usually I think that would be the Right Thing (e.g setting default values for -fieldsplit_option), but seeing as this is a purely debugging option then arguably you want to be able to set it globally.

I am erring on the side of keeping the current behaviour to be consistent with other options. What do you think?

But in my example here there are multiple levels of nesting, so identifying the default fallback is not obvious.

Personally I think having a single global value is the best approach.

@JHopeCollins

Copy link
Copy Markdown
Member Author

But in my example here there are multiple levels of nesting, so identifying the default fallback is not obvious.

Each OptionsManager will only read the options at their own level so any options_left above/below won't be identified.

Personally I think having a single global value is the best approach.

I'm not necessarily opposed. It'd be nice to have more specificity but this is just a debugging flag after all.

@connorjward

Copy link
Copy Markdown
Collaborator

But in my example here there are multiple levels of nesting, so identifying the default fallback is not obvious.

Each OptionsManager will only read the options at their own level so any options_left above/below won't be identified.

Personally I think having a single global value is the best approach.

I'm not necessarily opposed. It'd be nice to have more specificity but this is just a debugging flag after all.

Up to you. If you do want to leave it like this I would find a comment helpful effectively stating that intermediate flags are deliberately ignored.

@JHopeCollins

Copy link
Copy Markdown
Member Author

Personally I think having a single global value is the best approach.

I have come around to this. PETSc only tests for the "global" -options_left so trying to allow more specificity in nested solvers just confuses things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants