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

Update options.jl to use @for_petsc macro #126

Merged
merged 3 commits into from
Jul 8, 2021
Merged

Conversation

jkozdon
Copy link
Member

@jkozdon jkozdon commented Jul 8, 2021

Couple changes that happen with this PR

  • For PETSc arguments that have no input, nothing not true should be used, e.g., ksp_view = nothing.
  • Add parse_options for parsing command line arguments

@jkozdon
Copy link
Member Author

jkozdon commented Jul 8, 2021

@boriskaus and @nicoberlie: All the other PRs shouldn't have changed functionality of to code much. This one might impact you more, since it changes the behavior of the options.

In general, I am 100% willing to help fix the drama I created with my slew of PRs today. If you push out the update doc strings, I can try to fix any other drama I have created for you with #119

@boriskaus
Copy link
Collaborator

if you have a large number of options, it is often useful to be able to temporarily deactivate one of them, say snes_monitor. Is that still possible now that we should use snes_monitor=nothing rather than snes_monitor=true/false?

@jkozdon
Copy link
Member Author

jkozdon commented Jul 8, 2021

if you have a large number of options, it is often useful to be able to temporarily deactivate one of them, say snes_monitor. Is that still possible now that we should use snes_monitor=nothing rather than snes_monitor=true/false?

My thought is that options like snes_monitor have no values, so setting nothing in julia makes more sense to me (since nothing represents no value).

So we use this

S = PETSc.SNES{Float64}(MPI.COMM_SELF; 
        snes_rtol=1e-12, 
        snes_monitor=nothing,
        snes_converged_reason=nothing);

instead of

S = PETSc.SNES{Float64}(MPI.COMM_SELF; 
        snes_rtol=1e-12, 
        snes_monitor=true, 
        snes_converged_reason=true);

If you want to disable it, you just wouldn't include it.

If that's not going to work with your workflow I am fine undoing this.

@boriskaus
Copy link
Collaborator

yes I see.
The way I would normally do this in PETSc is to slightly change the name (e.g., call it -snes_monitor1 instead of -snes_monitor) to temporarily disable that. We can ofcourse do that here as well or, as you say, not include the option to start with. In a complex setup, however, you will have many options and this last case would require you to delete lines. That is why I believe setting it to false to disable is a handy feature to have. Is it possible to check if an option is nothing or false, and in case of false not include it in the options DB?

@jkozdon
Copy link
Member Author

jkozdon commented Jul 8, 2021

Yes, we can certainly do that. Will update the PR with this behavior.

Jeremy E Kozdon added 3 commits July 8, 2021 09:54
Also note that change of `true` to `nothing` for PETSc options that have
no argument
src/options.jl Show resolved Hide resolved
@jkozdon jkozdon merged commit f3eef64 into master Jul 8, 2021
@jkozdon jkozdon deleted the jek/update_options branch July 14, 2021 21:36
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