Skip to content
Snippets Groups Projects

Run!! Worklet Run!!

Merged Run!! Worklet Run!!
ollielo/vtk-m:worklet_run into master
3 unresolved threads
Merged Li-Ta Lo requested to merge ollielo/vtk-m:worklet_run into master
3 unresolved threads

Follow up !2842 (merged). Since we have UnknownCellSet, many Worklet::Run do not need to be templated on the CellSet type. We can keep UnknownCellSet all the way to Dispatcher/Invoker.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Li-Ta Lo requested review from @kmorel

    requested review from @kmorel

  • buildbot
    buildbot @buildbot ·
    Maintainer

    This merge request has been queued for testing. Test results may be viewed on:

    • Gitlab CI builders results can be viewed via the Pipelines for this merge request.
    • CDash links are also available in the Pipelines page
    • CDash (master)
    • Buildbot (master) (only visible from inside Kitware)

    Branch-at-master: e3fa8161

  • Kitware Robot
    Kitware Robot @kwrobot ·
    Owner

    Errors:

    • commit cf0127b3 is not allowed because the following files are not formatted according to the 'clang-format' check: vtkm/filter/entity_extraction/GhostCellRemove.cxx, vtkm/filter/entity_extraction/Threshold.cxx, vtkm/filter/entity_extraction/worklet/Threshold.h. Post a comment ending in the line Do: reformat to rewrite the MR source branch automatically.
    • commit e3fa8161 is not allowed because the following files are not formatted according to the 'clang-format' check: vtkm/filter/connected_components/worklet/CellSetConnectivity.h. Post a comment ending in the line Do: reformat to rewrite the MR source branch automatically.

    Please rewrite commits to fix the errors listed above (adding fixup commits will not resolve the errors) and force-push the branch again to update the merge request.

  • Li-Ta Lo
    Li-Ta Lo @ollielo ·
    Author Developer

    Do: format

  • Kitware Robot
    Kitware Robot @kwrobot ·
    Owner

    Errors:

    • @ollielo: the format command is not recognized at all.
  • Li-Ta Lo
    Li-Ta Lo @ollielo ·
    Author Developer

    Do: reformat

  • Kitware Robot
    Kitware Robot @kwrobot ·
    Owner

    This topic has been reformatted and pushed; please fetch from the source repository and reset your local branch to continue with further development on the reformatted commits.

  • Kitware Robot added 2 commits

    added 2 commits

    • 386c633d - simplify ThresholdWorklet::Run
    • b8c39f4f - de-template CellSetConnectivity::Run

    Compare with previous version

  • buildbot
    buildbot @buildbot ·
    Maintainer

    This merge request has been queued for testing. Test results may be viewed on:

    • Gitlab CI builders results can be viewed via the Pipelines for this merge request.
    • CDash links are also available in the Pipelines page
    • CDash (master)
    • Buildbot (master) (only visible from inside Kitware)

    Branch-at-master: b8c39f4f

  • Kenneth Moreland
    Kenneth Moreland @kmorel started a thread on an old version of the diff
vtkm/filter/connected_components/worklet/CellSetDualGraph.h
99 99
100 100 class CellSetDualGraph
101 101 {
102 public:
103 102 using Algorithm = vtkm::cont::Algorithm;
104 103
105 104 struct degree2
  • Kenneth Moreland
    Kenneth Moreland @kmorel started a thread on an old version of the diff
    • vtkm/filter/connected_components/worklet/GraphConnectivity.h
    69 69 {
    70 70 public:
    71 71 template <typename InputArrayType, typename OutputArrayType>
    72 void Run(const InputArrayType& numIndicesArray,
    73 const InputArrayType& indexOffsetsArray,
    74 const InputArrayType& connectivityArray,
    75 OutputArrayType& componentsOut) const
    72 static void Run(const InputArrayType& numIndicesArray,
    73 const InputArrayType& indexOffsetsArray,
    74 const InputArrayType& connectivityArray,
    75 OutputArrayType& componentsOut)
    76 76 {
    77 VTKM_IS_ARRAY_HANDLE(InputArrayType);
    78 VTKM_IS_ARRAY_HANDLE(OutputArrayType);
    • Comment on lines -77 to -78
      Kenneth Moreland
      Kenneth Moreland @kmorel ·
      Maintainer

      I recommend leaving these in. It's a good idea to use VTKM_IS_ARRAY_HANDLE on template arguments that are supposed to be some type of ArrayHandle for 2 reasons.

      1. It makes it much easier to diagnose problems when the method is used with arguments of the wrong type. If you pass in something other than an ArrayHandle, you will get some really weird, really long compile errors.
      2. It is a nice little piece of documentation for people like you and me reviewing this code. It is a succinct way to declare the type of the parameter.
    • Li-Ta Lo
      Li-Ta Lo @ollielo ·
      Author Developer

      Sure.

    • Li-Ta Lo changed this line in version 3 of the diff

      changed this line in version 3 of the diff

    • Please register or sign in to reply
  • Kenneth Moreland
    Kenneth Moreland @kmorel ·
    Maintainer

    Once the dashboard issues are resolved, +1

  • Li-Ta Lo added 1 commit

    added 1 commit

    • 2f62e8c2 - minor change based on code review

    Compare with previous version

  • buildbot
    buildbot @buildbot ·
    Maintainer

    This merge request has been queued for testing. Test results may be viewed on:

    • Gitlab CI builders results can be viewed via the Pipelines for this merge request.
    • CDash links are also available in the Pipelines page
    • CDash (master)
    • Buildbot (master) (only visible from inside Kitware)

    Branch-at-master: 2f62e8c2

  • Li-Ta Lo
    Li-Ta Lo @ollielo ·
    Author Developer

    Do: merge

  • Kitware Robot merged

    merged

  • Li-Ta Lo mentioned in commit e42b1d00

    mentioned in commit e42b1d00

  • Vicente Bolea
    Vicente Bolea @vbolea ·
    Owner

    The machine is working again, we had to reboot it.

  • Vicente Bolea
    Vicente Bolea @vbolea ·
    Owner

    Very coincidentally it seems that the windows build is reporting errors due to this changes https://open.cdash.org/viewBuildError.php?buildid=8101710

  • Li-Ta Lo mentioned in merge request !2850 (merged)

    mentioned in merge request !2850 (merged)

    • Please register or sign in to reply