August 2020 - petsc-dev - mailman3.cels.anl.gov

Re: [petsc-dev] Pause-for-approval Pipelines?
by Satish Balay 28 Aug '20

28 Aug '20

On Thu, 27 Aug 2020, Jed Brown wrote: > If you make an account, I can add you so you can see the settings > > https://readthedocs.org/projects/petsc/ > > Currently, it's Patrick, Vaclav, and me. Ok - account created now [via gitlab] so I'm sbalay at readthedocs.org Satish

1 0

Re: [petsc-dev] PETSc issue I cannot post combine WaitForCUDA(); inside PetscLogGpuTimeEnd();
by Barry Smith 28 Aug '20

28 Aug '20

> On Aug 28, 2020, at 10:26 AM, Stefano Zampini <stefano.zampini(a)gmail.com> wrote: > > > >> On Aug 28, 2020, at 5:18 PM, Barry Smith <bsmith(a)petsc.dev <mailto:[email protected]>> wrote: >> >> >> >>> On Aug 28, 2020, at 5:35 AM, Karl Rupp <rupp(a)iue.tuwien.ac.at <mailto:[email protected]>> wrote: >>> >>> Hi, >>> >>>> Since we cannot post issues (reported here https://forum.gitlab.com/t/creating-new-issue-gives-cannot-create-issue-get… <https://forum.gitlab.com/t/creating-new-issue-gives-cannot-create-issue-get…>) here is my issue so I don't forget it. >>>> I think >>>> err = WaitForCUDA();CHKERRCUDA(err); >>>> ierr = PetscLogGpuTimeEnd();CHKERRQ(ierr); >>>> should be changed to include WaitForCUDA() actually WaitForDevice() inside the PetscLogGpuTimeEnd(). >>>> Currently sometimes the WaitForCUDA() is missing in a few places resulting in bad timing. >>>> Also some _SeqCUDA() don't have the PetscLogGpuTimeEnd() and need to be fixed. >>>> The current model is a maintenance nightmare. >>>> Does anyone see a problem with making this change? >>> >>> I'm fine with this change, as the maintenance benefits outweigh the performance cost for typical use cases. >>> >>> I propose to also add the WaitForDevice(); at PetscLogGpuTimeBegin(). This will ensure that no previous GPU kernel executions spill over into the timed section. Karl, When synchronization is turned on the precious GPU kernels should always have their own WaitForDevice(), so are you concerned about buggy code that does not include WaitForDevice? >> >> Might this incur an extra overhead checking the device? Or will it always be true that if there are no outstanding kernels it will not go to the GPU and the check will return immediately? > > If we want to have a two barrier model, I propose we log the timing for waiting at the first barrier separately. >> >> Barry >> >>> >>> Best regards, >>> Karli >

1 0

Re: [petsc-dev] PETSc issue I cannot post combine WaitForCUDA(); inside PetscLogGpuTimeEnd();
by Stefano Zampini 28 Aug '20

28 Aug '20

> On Aug 28, 2020, at 5:18 PM, Barry Smith <bsmith(a)petsc.dev> wrote: > > > >> On Aug 28, 2020, at 5:35 AM, Karl Rupp <rupp(a)iue.tuwien.ac.at> wrote: >> >> Hi, >> >>> Since we cannot post issues (reported here https://forum.gitlab.com/t/creating-new-issue-gives-cannot-create-issue-get…) here is my issue so I don't forget it. >>> I think >>> err = WaitForCUDA();CHKERRCUDA(err); >>> ierr = PetscLogGpuTimeEnd();CHKERRQ(ierr); >>> should be changed to include WaitForCUDA() actually WaitForDevice() inside the PetscLogGpuTimeEnd(). >>> Currently sometimes the WaitForCUDA() is missing in a few places resulting in bad timing. >>> Also some _SeqCUDA() don't have the PetscLogGpuTimeEnd() and need to be fixed. >>> The current model is a maintenance nightmare. >>> Does anyone see a problem with making this change? >> >> I'm fine with this change, as the maintenance benefits outweigh the performance cost for typical use cases. >> >> I propose to also add the WaitForDevice(); at PetscLogGpuTimeBegin(). This will ensure that no previous GPU kernel executions spill over into the timed section. > > Might this incur an extra overhead checking the device? Or will it always be true that if there are no outstanding kernels it will not go to the GPU and the check will return immediately? If we want to have a two barrier model, I propose we log the timing for waiting at the first barrier separately. > > Barry > >> >> Best regards, >> Karli

1 0

Re: [petsc-dev] PETSc issue I cannot post combine WaitForCUDA(); inside PetscLogGpuTimeEnd();
by Barry Smith 28 Aug '20

28 Aug '20

> On Aug 28, 2020, at 5:35 AM, Karl Rupp <rupp(a)iue.tuwien.ac.at> wrote: > > Hi, > >> Since we cannot post issues (reported here https://forum.gitlab.com/t/creating-new-issue-gives-cannot-create-issue-get…) here is my issue so I don't forget it. >> I think >> err = WaitForCUDA();CHKERRCUDA(err); >> ierr = PetscLogGpuTimeEnd();CHKERRQ(ierr); >> should be changed to include WaitForCUDA() actually WaitForDevice() inside the PetscLogGpuTimeEnd(). >> Currently sometimes the WaitForCUDA() is missing in a few places resulting in bad timing. >> Also some _SeqCUDA() don't have the PetscLogGpuTimeEnd() and need to be fixed. >> The current model is a maintenance nightmare. >> Does anyone see a problem with making this change? > > I'm fine with this change, as the maintenance benefits outweigh the performance cost for typical use cases. > > I propose to also add the WaitForDevice(); at PetscLogGpuTimeBegin(). This will ensure that no previous GPU kernel executions spill over into the timed section. Might this incur an extra overhead checking the device? Or will it always be true that if there are no outstanding kernels it will not go to the GPU and the check will return immediately? Barry > > Best regards, > Karli

1 0

Re: [petsc-dev] PETSc issue I cannot post combine WaitForCUDA(); inside PetscLogGpuTimeEnd();
by Karl Rupp 28 Aug '20

28 Aug '20

Hi, > ï¿½ Since we cannot post issues (reported here > https://forum.gitlab.com/t/creating-new-issue-gives-cannot-create-issue-get…) > here is my issue so I don't forget it. > > ï¿½ I think > > ï¿½err ï¿½= WaitForCUDA();CHKERRCUDA(err); > ï¿½ierr = PetscLogGpuTimeEnd();CHKERRQ(ierr); > > should be changed to include WaitForCUDA() actually WaitForDevice() > inside the PetscLogGpuTimeEnd(). > > Currently sometimes the WaitForCUDA() is missing in a few places > resulting in bad timing. > > Also some _SeqCUDA() don't have the PetscLogGpuTimeEnd() and need to be > fixed. > > The current model is a maintenance nightmare. > > Does anyone see a problem with making this change? I'm fine with this change, as the maintenance benefits outweigh the performance cost for typical use cases. I propose to also add the WaitForDevice(); at PetscLogGpuTimeBegin(). This will ensure that no previous GPU kernel executions spill over into the timed section. Best regards, Karli

1 0

PETSc issue I cannot post combine WaitForCUDA(); inside PetscLogGpuTimeEnd();
by Barry Smith 28 Aug '20

28 Aug '20

Since we cannot post issues (reported here https://forum.gitlab.com/t/creating-new-issue-gives-cannot-create-issue-get… <https://forum.gitlab.com/t/creating-new-issue-gives-cannot-create-issue-get…>) here is my issue so I don't forget it. I think err = WaitForCUDA();CHKERRCUDA(err); ierr = PetscLogGpuTimeEnd();CHKERRQ(ierr); should be changed to include WaitForCUDA() actually WaitForDevice() inside the PetscLogGpuTimeEnd(). Currently sometimes the WaitForCUDA() is missing in a few places resulting in bad timing. Also some _SeqCUDA() don't have the PetscLogGpuTimeEnd() and need to be fixed. The current model is a maintenance nightmare. Does anyone see a problem with making this change? Thanks Barry

1 0

Re: [petsc-dev] Pause-for-approval Pipelines?
by Jed Brown 27 Aug '20

27 Aug '20

Satish Balay via petsc-dev <petsc-dev(a)mcs.anl.gov> writes: > On Thu, 27 Aug 2020, Satish Balay via petsc-dev wrote: > >> I don't understand how readthedocs is setup or how it works. And I think its the cause for most of the current issues. >> >> Its best to disable it completely for now - and see if most of the current issues go away or not > > > Ok I see the settings at https://gitlab.com/petsc/petsc/hooks > > The current triggers are: > > - push events > - tag push events > - merge request events. > > Disabled them for now. Okay, that had been set automatically. I activated push events to 'master'. From the looks of it, we could send a webhook directly from one of our CI jobs. E.g., if something in src/docs/ has changed. If you make an account, I can add you so you can see the settings https://readthedocs.org/projects/petsc/ Currently, it's Patrick, Vaclav, and me.

1 0

Re: [petsc-dev] Pause-for-approval Pipelines?
by Satish Balay 27 Aug '20

27 Aug '20

[want to respond to this part] On Thu, 27 Aug 2020, Barry Smith wrote: > Generally I like to pass the Pipeline before making a PR. This is api/web pipeline - and continues to work as before. > So the test on creating a new MR is annoying. There is no change wrt MR creation [wrt the merge_request pipelines feature] > Yes after the initial MR I might be able to release the paused job in lieu of starting pipelines fresh. Yes - and I think its is useful to switch form api/web to merge_request pipeline - once the MR is created. > It would be nice to send some pushes that don't trigger a pipeline start at all because I know I don't need one. Maybe that is possible, I'll need to investigate. Yes - this is useful - and it will avoid the extra 'blocked' pipelines listing [even-though they are mostly no-op -aka pre stage only - and should be ignored]. Satish

1 0

Re: [petsc-dev] Pause-for-approval Pipelines?
by Satish Balay 27 Aug '20

27 Aug '20

On Thu, 27 Aug 2020, Satish Balay via petsc-dev wrote: > I don't understand how readthedocs is setup or how it works. And I think its the cause for most of the current issues. > > Its best to disable it completely for now - and see if most of the current issues go away or not Ok I see the settings at https://gitlab.com/petsc/petsc/hooks The current triggers are: - push events - tag push events - merge request events. Disabled them for now. Satish

1 0

Re: [petsc-dev] Pause-for-approval Pipelines?
by Satish Balay 27 Aug '20

27 Aug '20

On Thu, 27 Aug 2020, Barry Smith wrote: > > > > On Aug 27, 2020, at 6:25 PM, Satish Balay <balay(a)mcs.anl.gov> wrote: > > > > On Thu, 27 Aug 2020, Satish Balay via petsc-dev wrote: > > > >> The above is hard for me to digest - so I think its best to frame this differently. > >> > >> 1. ReadTheDocs is not setup in .gitlab-ci.yml > >> > >> 2. We have merge_request pipelines - and web pipelines [api pipeline is similar to web] > >> > >> 3. I think both - web, merge_request pipelines trigger ReadTheDocs job > >> > >> 4. Web pipeline appears to get a 6th stage from the ReadTheDocs job [because both are run on the branch - and the hash matches] > >> > >> 5. Web pipelines are manually started. But MR pipelines are automatically started [I don't see an option to disable this auto start] > >> > >> 6. The ReadTheDocs job is run on the branch - even when triggered by merge_request pipeline. As the branch hash and the merge hash > >> are different - they are getting listed as 2 pipelines. > >> > >> And I think this separate pipeline listing by ReadTheDocs is causing grief with the MR webpage [it assumes the tests have passed] > >> > >> So the likely fixes: > >> > >> - ReadTheDocs triggered by MR pipeline should use the merge - and not the original branch [and not create a separate pipeline listing]. This should cut down the pipeline listing by half > >> - somehow disable auto start of MR pipelines [this should eliminate the remaining pipeline listings. > > > > https://gitlab.com/petsc/petsc/-/issues/360#note_402053765 > > > > Ok - I understand the 'hanging' pipeline issue better now. > > > > - Multiple ReadTheDocs jobs can be triggered on the same commit. > > > > - perhaps one from the web pipeline [when one starts this manually] > > But but but, Jed said we had to enable MR pipelines or readthedocs won't work. So readthedocs cannot be triggered by a manual pipeline > > I would prefer the model that we in our scripts contact readthedocs to request they do a build, not having it automatically triggered by changes in GitLab, they may or may not support this but if they do it is hidden from Jed. I don't understand how readthedocs is setup or how it works. And I think its the cause for most of the current issues. Its best to disable it completely for now - and see if most of the current issues go away or not Sure - the auto-start will keep populating the pipeline queue for every push on a MR (if readthedocs is disabled - it should be cut down by half) - but that's the current trade-off. Satish > > > > - irrespective of the above - MR pipeline is automatically triggered. And this triggers another ReadTheDocs pipeline (and this pipeline is run on the branch - not a merge of branch+master) > > - one might want to start a new MR pipeline with a merge with updates in master > > > > ReadTheDocs sees the second pipeline as duplicate - and errors out. > > > > - for some reason gitlab does not get this error message or misinterprets it - and the job is listed as 'external:pending' > > > > Satish >

1 0