Mesh partitioning with zoltan instead of metis. #5514

niveshd · 2017-11-22T09:45:42Z

Zoltan is used for graph partitioning. The following page for zoltan's guide has more information. http://www.cs.sandia.gov/zoltan/ug_html/ug_alg_phg.html

Implementation:
4 query functions, as need by zoltan, are added in sparsity_tools.cc
Additional option has been added to "partition_triangulation" function to choose between "metis" or "zoltan"

davydden

very nice 👍 please also add a changelog in doc/news/changes/minor

davydden · 2017-11-22T09:47:06Z

include/deal.II/grid/grid_tools.h

@@ -28,6 +28,8 @@
 #include <deal.II/grid/tria_accessor.h>
 #include <deal.II/grid/tria_iterator.h>
 #include <deal.II/hp/dof_handler.h>
+//Needed for partition_triangulation


we usually don't document headers, you can remove this line

davydden · 2017-11-22T09:49:46Z

include/deal.II/lac/sparsity_tools.h

+   */
+  enum Partitioner
+  {
+    metis = 0,


please document those, just add

/** * Use METIS partitioner. */

and

/** * Use Zoltan partitioner. */

davydden · 2017-11-22T09:50:27Z

source/grid/grid_tools.cc

@@ -56,6 +56,9 @@
 #include <list>
 #include <set>

+//Needed for partition_triangulation


that can be removed.

davydden · 2017-11-22T09:53:03Z

source/lac/sparsity_tools.cc

@@ -39,15 +39,270 @@ extern "C"
 DEAL_II_ENABLE_EXTRA_DIAGNOSTICS
 #endif

+#include <zoltan_cpp.h>


we need to be more careful here as Trilinos might not be compiled with Zoltan package, please add Zoltan to
this line https://github.com/dealii/dealii/blob/master/cmake/configure/configure_2_trilinos.cmake#L58 and guard these includes and code in

#ifdef DEAL_II_TRILINOS_WITH_Zoltan ... #endif

it would probably be better to use STRING(TOUPPER "${_lib}" _lib_upper), something like

FOREACH(_optional_module ROL Zoltan ) ITEM_MATCHES(_module_found ${_optional_module} ${Trilinos_PACKAGE_LIST}) IF(_module_found) MESSAGE(STATUS "Found ${_optional_module}") STRING(TOUPPER "${_optional_module}" _optional_module_upper) SET(DEAL_II_TRILINOS_WITH_${_optional_module_upper} ON) ELSE() MESSAGE(STATUS "Module ${_optional_module} not found!") ENDIF() ENDFOREACH()

so that we can write

DEAL_II_TRILINOS_WITH_ZOLTAN

consistent with all other macros.

Also guard the include with DEAL_II_DISABLE_EXTRA_DIAGNOSTICS

davydden · 2017-11-22T09:55:59Z

source/lac/sparsity_tools.cc

+    //Query functions for partition_zoltan
+    int get_number_of_objects(void *data, int *ierr)
+    {
+      SparsityPattern *graph = (SparsityPattern *)data;


please use

SparsityPattern *graph = static_cast<SparsityPattern *>(data);

same below.

davydden · 2017-11-22T09:57:37Z

source/lac/sparsity_tools.cc

+
+      for (int i=0; i < num_obj; ++i)
+        {
+


davydden · 2017-11-22T09:57:52Z

source/lac/sparsity_tools.cc

+
+          for ( SparsityPattern::iterator col=graph->begin(i);
+                col < graph->end(i); ++col )
+            {


you can remove this bracket and write if directly under.

davydden · 2017-11-22T09:57:59Z

source/lac/sparsity_tools.cc

+              if ( i != col->column() )
+                {
+                  *nextNborGID++ = col->column();
+                  //std::cout << "Neighbours " << globalID[i] << " : " << *(nextNborGID-1) << std::endl;


davydden · 2017-11-22T09:58:39Z

source/lac/sparsity_tools.cc

+      //Clean up
+      delete zz;
+
+      //Terminate MPI in the main program!


you can remove this.

davydden · 2017-11-22T09:59:45Z

tests/zoltan/CMakeLists.txt

+INCLUDE(../setup_testsubproject.cmake)
+PROJECT(testsuite CXX)
+
+DEAL_II_PICKUP_TESTS()


guard this in

IF(DEAL_II_WITH_TRILINOS AND DEAL_II_TRILINOS_WITH_Zoltan) DEAL_II_PICKUP_TESTS() ENDIF()

davydden · 2017-11-22T10:09:44Z

tests/zoltan/zoltan_01.cc

+    {
+      //Initialize MPI and Zoltan
+      float version;
+      MPI::Init(argc, argv);


we usually use Utilities::MPI::MPI_InitFinalize mpi_initialization (argc, argv); auxiliary class to do initialization.

you would do

Utilities::MPI::MPI_InitFinalize mpi_initialization (argc, argv); test<1> (); test<2> (); test<3> ();

and the destructor of MPI_InitFinalize will take care of calling finalize.

davydden · 2017-11-22T10:16:07Z

tests/zoltan/zoltan_01.cc

+      //Initialize MPI and Zoltan
+      float version;
+      MPI::Init(argc, argv);
+      Zoltan_Initialize(argc, argv, &version);


@dealii/dealii should not that go into constructor of MPI_InitFinalize in base/mpi.cc?

Yes it should. That's what we do for PETSc and TBB

davydden · 2017-11-22T10:46:59Z

include/deal.II/lac/sparsity_tools.h

+  /**
+   * Enumerator with options for partitioner
+   */
+  enum Partitioner


please use strong enumerators, i.e. enum class Partitioner, see https://stackoverflow.com/questions/18335861/why-is-enum-class-preferred-over-plain-enum

davydden · 2017-11-22T11:02:01Z

tests/zoltan/zoltan_01.cc

+
+
+
+// check GridTools::partition_triangulation


you probably use some other test as an example, it's good to state it here so that in case something goes wrong
in years to come, it's easier to debug.

You can also be a bit more specific by saying

check GridTools::partition_triangulation using Zoltan

davydden · 2017-11-22T11:02:30Z

tests/zoltan/zoltan_01.cc

+#include <deal.II/grid/tria_iterator.h>
+#include <deal.II/grid/grid_tools.h>
+#include <deal.II/grid/grid_generator.h>
+#include <zoltan_cpp.h>


this should not be needed, i think

davydden · 2017-11-22T11:04:58Z

tests/zoltan/zoltan_01.cc

+{
+  initlog();
+
+  try


that was there in another test, i guess, but you can remove try/catch thing.

Rombur

This is great! Thanks for the PR

Rombur · 2017-11-22T13:52:16Z

source/lac/sparsity_tools.cc

+      Zoltan *zz = new Zoltan(MPI::COMM_SELF);
+
+      //Check if zoltan object is initialized
+      Assert( zz != NULL , ExcEmptyObject() );


Use nullptr instead of NULL

Rombur · 2017-11-22T13:54:37Z

tests/zoltan/zoltan_01.cc

+  GridGenerator::hyper_cube (triangulation);
+  triangulation.refine_global (5-dim);
+
+  // subdivideS into 5 subdomains


typo subdivideS

Rombur · 2017-11-22T13:55:38Z

tests/zoltan/zoltan_01.cc

+      //Initialize MPI and Zoltan
+      float version;
+      MPI::Init(argc, argv);
+      Zoltan_Initialize(argc, argv, &version);


Yes it should. That's what we do for PETSc and TBB

Rombur · 2017-11-22T14:07:21Z

source/lac/sparsity_tools.cc

+      Assert( zz != NULL , ExcEmptyObject() );
+
+      //General parameters
+      zz->Set_Param( "LB_METHOD", "GRAPH" );  //graph based partition method (LB-load balancing)


Is this the only partitioning method that you plan on using?

I chose graph partitioners based on the application and ease of implementation. Even with this one partitioner, there are different packages available ( set by using GRAPH_PACKAGE). I chose the default one.

I have not considered using geometric and simple partitioner (uses no connectivity info.) here. That leaves us with graph partitioners and hyper graph partitioners. In order to use hyper graph partitioners, other query functions have to be written and is in general more expensive than graph option.

davydden · 2017-11-22T22:08:18Z

./run-tests

bangerth · 2017-11-24T04:35:57Z

Oh, I so look forward to seeing this included in deal.II! Thank you very much for this contribution!

Right now, the build fails with this error message:

In file included from source/lac/unity_1.cc:5:0:
/home/bob/source/source/lac/sparsity_tools.cc:44:24: fatal error: zoltan_cpp.h: No such file or directory
 #include <zoltan_cpp.h>
                        ^

bangerth

Great work, thanks for tackling this!

bangerth · 2017-11-24T04:52:19Z

include/deal.II/grid/grid_tools.h

+   * METIS when multiple partitions are required.
+   *
+   * To use ZOLTAN partitioner, user has to take care of initialization and
+   * termination of MPI environment.


I think this has been discussed in another comment already, but we should really do this in the same place as we initialize MPI and PETSc.

indeed, the tests should use Utilities::MPI::MPI_InitFinalize mpi_initialization (argc, argv);, whereas we should move Zoltan MPI init into constructor of this class, see base/mpi.cc. See how it's done for other libraries, and duplicate this for Zoltan with the preprocessor guard.

sorry, i first commented and only then saw that @niveshd already did the changes. I think we can simply remove this line as it is not the case anymore.

Right. That's really what I meant to point out -- that the sentence should just be removed.

bangerth · 2017-11-24T04:52:43Z

include/deal.II/grid/grid_tools.h

+   * METIS is the default partitioner.
+   *
+   * Assuming METIS is chosen as partitioner, this function will generate an error in case METIS
+   * is not installed unless @p n_partitions is one. I.e., you can write a program so that it runs


Can you duplicate this paragraph also for the case that ZOLTAN has been chosen?

bangerth · 2017-11-24T04:55:44Z

include/deal.II/grid/grid_tools.h

-   * the neighborship graph, and METIS will not usually cut important
-   * connections in this case. However, if there are vertices in the mesh
-   * where many cells (many more than the common 4 or 6 in 2d and 3d,
+   * the neighborship graph, and METIS, when chosen as the partitioner,


I think this comment (and maybe also the one in line 1395/1404) is not meant to be specific to METIS, just about partitioning in general. It may be useful to just replace "METIS" by "partitioning algorithm" or something similar. This would save us the hassle to update the documentation again if we ever choose to add a third or fourth partitioning algorithm.

bangerth · 2017-11-24T04:56:56Z

include/deal.II/lac/sparsity_tools.h

+   * Assuming METIS is chosen as partitioner, this function will generate an error if
+   * METIS is not installed unless @p n_partitions is one. I.e., you can write a program
+   * so that it runs in the single-processor single-partition case without METIS installed,
+   * and only requires METIS when multiple partitions are required.


same here -- say what happens if you choose ZOLTAN but ZOLTAN has not been found.

bangerth · 2017-11-24T05:03:32Z

source/lac/sparsity_tools.cc

+
+      // If metis returns normally, an error code METIS_OK=1 is returned from
+      // the above functions (see metish.h)
+      AssertThrow (ierr == 1, ExcMETISError (ierr));


We all already knew how crappy METIS is, but this just reinforces my opinion -- using 1 as the OK return code...

bangerth · 2017-11-24T05:06:05Z

source/lac/sparsity_tools.cc

+      SparsityPattern *graph = static_cast<SparsityPattern *>(data);
+      *ierr = ZOLTAN_OK;
+
+      Assert ( numEdges != nullptr , ExcEmptyObject() );


same here, and probably in all of the places below where you use ExcEmptyObject.

bangerth · 2017-11-24T05:09:07Z

source/lac/sparsity_tools.cc

+                           std::vector<unsigned int> &partition_indices)
+    {
+      //MPI environment must have been initialized by this point.
+      Zoltan *zz = new Zoltan(MPI::COMM_SELF);


use

std::unique_ptr<Zoltan> zz = std_cxx14::make_unique<Zoltan>(MPI::COMM_SELF));

here. You then don't need the delete below, and you also don't need the check for nullptr in the next line -- new (or make_unique) will throw an exception if it can't allocate the memory.

Further, this should probably be MPI_COMM_SELF.

Oh, yes, good eyes! The MPI C++ bindings are going to go away.

bangerth · 2017-11-24T05:09:54Z

source/lac/sparsity_tools.cc

+                                 export_to_part);
+
+      //check for error code in partitioner
+      Assert( rc == ZOLTAN_OK , ExcEmptyObject() );


That seems the wrong exception object.

bangerth · 2017-11-24T05:10:48Z

source/lac/sparsity_tools.cc

+      //check for error code in partitioner
+      Assert( rc == ZOLTAN_OK , ExcEmptyObject() );
+
+      Assert (partition_indices.size() != 0, ExcZero());


This would indicate an error in Zoltan. I would map that to ExcInternalError.

it's actually input to the function. We already have

Assert (partition_indices.size() == sparsity_pattern.n_rows(), ExcInvalidArraySize (partition_indices.size(), sparsity_pattern.n_rows()));

so this one can simply go.

bangerth · 2017-11-24T05:12:09Z

source/lac/sparsity_tools.cc

+      //copy from export_to_part to partition_indices
+      for (int i=0; i < num_export; i++)
+        {
+          partition_indices[export_local_ids[i]] = export_to_part[i];


Is there a guarantee that export_local_ids[i] loops over all indices? If so, then we wouldn't need to fill the array by zeros immediately above. If there is no such guarantee, then filling the array with invalid indices might be the better choice.

It is not intended to go through all the indices, rather only the ones that changed the partitions. This is why i set the default partition id to be zero.

Hm, I don't think I understand. partition_indices is only an output variable -- as far as I can see, it's previous contents (if any) are not actually given to Zoltan, right? In that case, I would expect that Zoltan provides as its output a partition index for each node of the graph, and so export_local_ids[i] should loop through all nodes. If that were not the case, what partition is assigned to graph nodes that are not listed in export_local_ids? Right now, it is zero, but why is that the correct answer?

Initially all of them belong to part 0. After partitioning, export_local_ids only has ids whose part ids(within each process) have changed. I will try if I can do it directly as you suggested!

After partitioning, export_local_ids only has ids whose part ids(within each process) have changed.

i think it's OK if you simply add this as a note above this line so that anyone reading the code would understand.

p.s. from this code, it does not look like you can use partition_indices directly in Zoltan as the output is ZOLTAN_ID_PTR export_local_ids=nullptr. So it looks like Zoltan internally allocates memory and hopefully also cleans it up in destructor...

That is true. Everything is allocated based on the query functions passed to zoltan

Yes, just putting the reasoning into a comment is good enough -- simply state that Zoltan only provides information about nodes whose partition is != 0.

davydden

only cosmetic suggestions.

davydden · 2017-11-24T06:17:12Z

include/deal.II/base/config.h.in

@@ -57,6 +57,7 @@
 #cmakedefine DEAL_II_WITH_TRILINOS
 #cmakedefine DEAL_II_WITH_UMFPACK
 #cmakedefine DEAL_II_WITH_ZLIB
+#cmakedefine DEAL_II_TRILINOS_WITH_ZOLTAN


nitpick, but could you please move this after the line #cmakedefine DEAL_II_TRILINOS_WITH_ROL where other Trilinos-related things are defined.

no problem. Thanks!

davydden · 2017-11-24T06:20:09Z

include/deal.II/grid/grid_tools.h

@@ -1361,25 +1362,33 @@ namespace GridTools
                                             DynamicSparsityPattern             &connectivity);

  /**
-   * Use the METIS partitioner to generate a partitioning of the active cells
+   * Use METIS or ZOLTAN partitioner to generate a partitioning of the active cells


to make it extendable, it's probably easier to say

Use graph partitioner...

and below

Currently the user can choose between METIS (default) and ZOLTAN partitioner. If deal.II was not
compiled with the corresponding package, then the function will generate en error unless .... .

this is in line of what @bangerth suggested https://github.com/dealii/dealii/pull/5514/files#r152897649

davydden · 2017-11-24T06:34:15Z

the tester error might have to do with Trilinos version 12.4.2 not having the header zoltan_cpp.h. We had similar issues with ROL #5464 . I suggest that for now we add the following right after optional modules are being checked:

    IF(${DEAL_II_TRILINOS_WITH_ZOLTAN})
       IF(TRILINOS_VERSION VERSION_LESS x.y.z)  // <---- put the version you know works!
         MESSAGE(STATUS "Zoltan interface is disabled."
           "deal.II will not support Zoltan interface if Trilinos version is less "
           "than x.y.z. Trilinos version found: \"${TRILINOS_VERSION}\".") // <---- same here
           SET(DEAL_II_TRILINOS_WITH_ZOLTAN OFF)
       ENDIF()
     ENDIF()

~~later we can refine this and see what's going on... @dealii/dealii anyone disagrees?~~

davydden · 2017-11-24T06:39:50Z

hm, there is something else going on here, clang tester also uses 12.4.2 but it compiles fine!.. there must be some additional configurational difference in Trilinos that makes this header absent...

davydden · 2017-11-24T06:43:16Z

maybe as a temporary solution do something like:

DEAL_II_FIND_FILE(ZOLTAN_CPP zoltan_cpp.h
  HINTS ${TRILINOS_INCLUDE_DIR}
  NO_DEFAULT_PATH NO_CMAKE_ENVIRONMENT_PATH NO_CMAKE_PATH
  NO_SYSTEM_ENVIRONMENT_PATH NO_CMAKE_SYSTEM_PATH NO_CMAKE_FIND_ROOT_PATH
  )

IF(NOT EXISTS ${ZOLTAN_CPP})
  SET(DEAL_II_TRILINOS_WITH_ZOLTAN OFF)
  MESSAGE(STATUS "Zoltan interface is disabled. Could not locate zoltan_cpp.h"
ENDIF()

bangerth · 2017-11-24T15:23:23Z

But if zoltan_cpp.h can't be found, how did cmake determine that Zoltan is available? Shouldn't that just have led to us not using Zoltan at all? (In which case we should never have tried to #include that file to begin with?)

davydden · 2017-11-24T15:25:41Z

But if zoltan_cpp.h can't be found, how did cmake determine that Zoltan is available?

that's based on ITEM_MATCHES(_module_found ${_optional_module} ${Trilinos_PACKAGE_LIST}) and yes, gcc build of tester does have Zoltan module, but for whatever reason the header is not there. Personally, I don't know why would this happen, but checking for a header in addition to module list seems ok for starters until someone figures out what is going on with Trilinos.

bangerth · 2017-11-24T15:38:04Z

Oh, how bizarre. @tjhei -- do you have an idea why that would be so?

We've had this issue before that Trilinos said it had a sublibrary but then didn't install everything that is necessary for it. I wonder if we should just hard-error out in that case in cmake?

davydden

thanks for working through comments, just a few from my side.

davydden · 2017-11-25T10:08:27Z

source/lac/sparsity_tools.cc

+
+      return graph->n_rows();
+    }
+#endif


you don't have to put #endif here, just put it right before void partition_zoltan. Then you can remove all those #ifdef / #endif in between.

davydden · 2017-11-25T10:08:57Z

source/lac/sparsity_tools.cc

+          localID[i] = i;     //same as global ids
+        }
+
+      return;


no need for return as it's a void function. same below.

davydden · 2017-11-25T10:12:32Z

source/lac/sparsity_tools.cc

+      // installed and detected
+#ifndef DEAL_II_TRILINOS_WITH_ZOLTAN
+      (void)sparsity_pattern;
+      AssertThrow (false, ExcInternalError());


please change the asset type, for METIS we have

AssertThrow (false, ExcMETISNotInstalled());

where ExcMETISNotInstalled is a custom assert. You can check include/deal.II/lac/sparsity_tools.h line 218 to see how it's defined and copy-paste the same for Zoltan.

davydden · 2017-11-25T10:13:06Z

source/lac/sparsity_tools.cc

+
+      //copy from export_to_part to partition_indices, whose part ids have changed.
+      for (int i=0; i < num_export; i++)
+    {


please run indentation script.

davydden · 2017-11-25T10:15:45Z

@niveshd you also need to rebase as we have merge conflicts now. Roughly,

git checkout master
git pull
git checkout zoltanPartition
git rebase master

davydden · 2017-11-25T11:14:17Z

source/lac/sparsity_tools.cc

+#else
+
+      //MPI environment must have been initialized by this point.
+      std::unique_ptr<Zoltan> zz = std::make_unique<Zoltan>(MPI_COMM_SELF);


please use std_cxx14::make_unique, this is itnernal namespace in deal.II which will be equal to std:: if c++14 is there or will fallback to boost otherwise. That shall fix compiler errors on the tester.

davydden · 2017-11-25T11:16:50Z

source/lac/sparsity_tools.cc

+
+      //set global degrees of freedom
+      auto n_dofs = graph->n_rows();
+      for (int i=0; i < n_dofs; i++)


in order to avoid warnigns

/home/bob/source/source/lac/sparsity_tools.cc:165:23: warning: comparison of integers of different signs: 'int' and 'unsigned int' [-Wsign-compare] for (int i=0; i < n_dofs; i++)

do: for (unsigned int i = 0; ....

davydden · 2017-11-25T11:23:12Z

source/lac/sparsity_tools.cc

+      Assert( nextNborProc != nullptr , ExcInternalError() );
+
+      //Loop through rows corresponding to indices in globalID implicitly
+      for (int i=0; i < num_obj; ++i)


for (SparsityPattern::size_type i=0; i < static_castSparsityPattern::size_type>(num_obj); ++i) to avoid

/home/bob/source/source/lac/sparsity_tools.cc:219:20: warning: comparison of integers of different signs: 'int' and 'size_type' (aka 'unsigned int') [-Wsign-compare] if ( i != col->column() ) ~ ^ ~~~~~~~~~~~~~

davydden · 2017-11-25T11:28:09Z

@tjhei look like the gcc tester does NOT have correct trilinos around

-- Could not find a sufficient Trilinos installation: Trilinos has to be configured with the same MPI configuration as deal.II.
-- DEAL_II_WITH_TRILINOS has unmet external dependencies.

davydden · 2017-11-25T11:37:52Z

cmake/configure/configure_2_trilinos.cmake

@@ -55,26 +55,18 @@ MACRO(FEATURE_TRILINOS_FIND_EXTERNAL var)
    ENDFOREACH()

    FOREACH(_optional_module


@bangerth @niveshd I know how to fix the error. The problem is that we should check optional modules after we do all the check for trilinos being ok. There are a lot below which may eventually disable trilinos. As a result we first might have DEAL_II_TRILINOS_WITH_XYZ set but then we actually disable Trilinos altogether.

@niveshd please move the whole FOREACH(_optional_module part right after CHECK_MPI_INTERFACE(TRILINOS ${var}) in this file so that we have:

IF(${var}) FOREACH(_optional_module .... ENDFOREACH() ENDIF()

this way we look for optional modules and enable them only if Trilinos has met all the rerquired dependencies above this line. Otherwise ${var} will be set to FALSE at some of the checks.

p.s. DEAL_II_FIND_FILE(ZOLTAN_CPP zoltan_cpp.h shall not be needed as from the tester we see:

-- Found Zoltan -- Found MueLu -- Found ZOLTAN_CPP -- Could not find a sufficient Trilinos installation: Trilinos has to be configured with the same MPI configuration as deal.II. -- DEAL_II_WITH_TRILINOS has unmet external dependencies.

davydden · 2017-11-25T16:44:05Z

Could please try rebasing again? There are still conflicts in the CMake related file. You wou need to force push after that: git push -f

davydden · 2017-11-25T18:51:51Z

Ps. Since u modify that cmake file more than once, it might be easier to simply merge in “master” to your branch and resolve the conflict. After that you can check that everything still works and tests passes and then squash into a single commit and push force.

niveshd · 2017-11-25T19:41:38Z

@davydden i tried rebasing and push -f. But the conflict is not cleared. I think it's because I am trying to modify some old stuff. Should I just separate ROL and Zoltan in that loop which checks for these libraries.

davydden · 2017-11-25T20:45:33Z

@niveshd

i tried rebasing and push -f. But the conflict is not cleared.

Are you sure you did

git checkout master
git pull
git checkout zoltanPartition

?

I mean you local master should be up-to-date with the dela.ii's master before you try to do anything.

In your case I think it's easier to simply merge master to your branch, i.e.

git merge master

That will ask you to resolve conflict. Once you are done, you shall add conflicting files

git add # probably: configure_2_trilinos.cmake

and then commit

git commit

The only changes to configure_2_trilinos.cmake should be related to that FOREACH(_optional_module part and nothing else.

davydden · 2017-11-25T20:47:24Z

p.s. the whole reason for conflict is that after your branched off the master to start working with Zoltan, someone modified the same file as you in such a way that there is a conflict. So that's why you first need to get those changes locally, simplest is to pull the local master branch.

davydden · 2017-11-25T20:48:09Z

Should I just separate ROL and Zoltan in that loop which checks for these libraries.

no, certainly not.

davydden · 2017-11-26T00:37:18Z

@niveshd i tried resolving from the web-interface but it did not work. (empty commit and conflict is still there). Try squashing locally and push --force. Maybe GitHub is confused somehow...

davydden · 2017-11-26T00:44:49Z

@niveshd i created #5530 so hopefully when it's merged, GitHub will not be confused with conflicts in your branch.

davydden · 2017-11-26T16:35:35Z

@niveshd Thanks for squashing.

Something is wierd with that cmake/configure/configure_2_trilinos.cmake. Now it shows that you are modifying the whole thing... I am puzzled...

Maybe try:

git checkout master -- cmake/configure/configure_2_trilinos.cmake # will checkout this file from master
// do the changes you need to this file
git commit -am "fix conflict"
git push

niveshd · 2017-11-26T17:32:31Z

@davydden , i tried what you said. but strangely it still shows the problem. Probably the reason it says i tried to modify the whole thing was because i copied the cmake file you sent me directly.

Zoltan is used for graph partitioning. The following page for zoltan's guide has more information. http://www.cs.sandia.gov/zoltan/ug_html/ug_alg_phg.html Implementation: - 4 query functions, as need by zoltan, are added in sparsity_tools.cc - additional option has been added to "partition_triangulation" function to choose between "metis" or "zoltan"

tamiko · 2017-11-26T17:53:21Z

@niveshd I have resolved the merge conflict, split into several commits (retaining you as author) and rebased to master.

I have pushed all changes to your remote branch niveshd:zoltanPartition - please pull locally and check whether everything is allright :-)

davydden · 2017-11-26T18:04:15Z

@tamiko thanks for fixing the issue. Most likely the branch was not rebased on master...

@niveshd you won't be able to pull due to conflicts locally, so you need to reset hard to the remote (your github) branch.

davydden · 2017-11-26T19:22:16Z

p.s. @niveshd i know why the whole rebase was no doing anything, your master branch https://github.com/niveshd/dealii/commits/master also has some Zoltan related changes. So that means when you did git pull on master you must have had some conflicts as your master and dealii's master diverged. Or perhaps you set your master to track master from your remote (github) so then no changes were pulled at all. Consequently git rebase master does not do much. One could have, of course, rebased on top of the remote master branch from dealii:

git fetch origin
git rebase origin/master

My advice is to never commit anything to master so that you can always easily pull the upstream branch. It's always better to work on the dedicated feature branches.

bangerth · 2017-11-26T23:19:43Z

Fantastic -- let's see how this works!

davydden added the Triangulation label Nov 22, 2017

davydden reviewed Nov 22, 2017

View reviewed changes

Rombur reviewed Nov 22, 2017

View reviewed changes

davydden added the WIP label Nov 23, 2017

bangerth reviewed Nov 24, 2017

View reviewed changes

davydden approved these changes Nov 24, 2017

View reviewed changes

davydden approved these changes Nov 25, 2017

View reviewed changes

davydden reviewed Nov 25, 2017

View reviewed changes

davydden mentioned this pull request Nov 26, 2017

cmake: check trilinos optional modules at the very end #5530

Closed

niveshd force-pushed the zoltanPartition branch 2 times, most recently from 8f7176e to e61e635 Compare November 26, 2017 08:59

niveshd added 4 commits November 26, 2017 11:45

CMake: Add check for Zoltan to trilinos configuration

af9ab46

add zoltan tests

4cc90f7

add changelog entry

3316c96

tamiko force-pushed the zoltanPartition branch from a60001d to 3316c96 Compare November 26, 2017 17:51

davydden approved these changes Nov 26, 2017

View reviewed changes

bangerth merged commit 2052289 into dealii:master Nov 26, 2017

This was referenced Nov 26, 2017

Tell users to build with ZOLTAN. #5533

Merged

Switch parallel::shared::Triangulation default to Zoltan #5534

Closed

niveshd deleted the zoltanPartition branch November 28, 2017 09:05

		@@ -55,26 +55,18 @@ MACRO(FEATURE_TRILINOS_FIND_EXTERNAL var)
		ENDFOREACH()

		FOREACH(_optional_module

Mesh partitioning with zoltan instead of metis. #5514

Mesh partitioning with zoltan instead of metis. #5514

Conversation

niveshd commented Nov 22, 2017

davydden left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rombur left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davydden commented Nov 22, 2017

bangerth commented Nov 24, 2017

bangerth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davydden Nov 24, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davydden Nov 24, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davydden left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davydden Nov 24, 2017 • edited

Choose a reason for hiding this comment

davydden commented Nov 24, 2017 • edited

davydden commented Nov 24, 2017

davydden commented Nov 24, 2017

bangerth commented Nov 24, 2017

davydden commented Nov 24, 2017

bangerth commented Nov 24, 2017

davydden left a comment

Choose a reason for hiding this comment

davydden Nov 25, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davydden Nov 24, 2017 •

edited

davydden Nov 24, 2017 •

edited

davydden Nov 24, 2017 •

edited

davydden commented Nov 24, 2017 •

edited

davydden Nov 25, 2017 •

edited

davydden Nov 25, 2017 •

edited