On Thu, 28 Jun 2018 at 18:51, Smith, Barry F. <[email protected]> wrote: > > > I am having trouble tracking the following issue (and people's opinion on > it) in the pull requests so I am starting an email thread to discuss the > issue. > > Currently - (this is my understanding) > > 1) PetscObjectViewFromOptions() is very slow > 2) Many calls are made to this routine directly from solvers (such as KSP) > > because of this there is a configure option to turn off checking of > PetscObjectViewFromOptions > > In pull request 1005 Matt moved the PetscObjectViewFromOptions() routines out > of KSPSolve() and put them in KSPSetFromOptions(). > > In pull request 1006 Lisandro proposes moving the configure option to turn > off checking of a PetscObjectViewFromOptions() to a runtime option. > > Here is my opinion, Matt is correct and all the PetscObjectViewFromOptions() > calls should be moved out of the solver code. There should be no option > either configure or runtime to turn off PetscObjectViewFromOptions() (I put > it in as a stop gap for some users who needed it). > > Comments, suggestions? >
My opinion is that Matt's approach, while technically sound, incorporates much more complexity. And if we are going to follow this model, it has to be done everywhere, that is, complexity should be propagated to the other components, and someone has to do it, and do it before next release, because we don't want PETSc to slowly become "XXXX" (XXXX=competing project we all know, but I'm being polite to not mention explicitly). About the "XXXViewFromOptions" is slow, we still don't have even a rough number about how much this hurts. And yet, we go for a much more complicated model. And as Matt's PR is not properly split in two (PCPATCH + changes to XXXViewFromOptions), then we just lost our way to actually assess the issue within KSP. Maybe all what we need is a hash table. And if a hash table is not enough, then yes, I declare myself defeated. PR 1006 is just my attempt to move from configure time to runtime. And I claim that all the issues in PCPATCH could be hot-fixed by the time being by a push/pop to the noviewer flag at PCAppy. But rather than going step by step, and decide later on the best way to handle XXXViewFromOptions(), now we have this gigantic PR that changes the core KSP infrastructure, and I know it is going to be merged as it is, and I know in advance I'm already defeated. Let it be, Let it be... -- Lisandro Dalcin ============ Research Scientist Computer, Electrical and Mathematical Sciences & Engineering (CEMSE) Extreme Computing Research Center (ECRC) King Abdullah University of Science and Technology (KAUST) http://ecrc.kaust.edu.sa/ 4700 King Abdullah University of Science and Technology al-Khawarizmi Bldg (Bldg 1), Office # 0109 Thuwal 23955-6900, Kingdom of Saudi Arabia http://www.kaust.edu.sa Office Phone: +966 12 808-0459
