Update points example #212

pkestene · 2023-04-30T09:38:08Z

Update points example

Following up on issue #211

Proposed changes:

provide a helper executable to generate example point data
alos add the brick connectivity (with a slightly different treatment, just for illustrative purpose)
use MPI-IO to create the point data file (a single file for all MPI processes).

Currently this can only be merged when companion MR cburstedde/libsc#128 which update MPI-IO routines in libsc is merge to.
The executable that generate point data file need these mpi-io routines in libsc.

cburstedde · 2023-07-04T06:54:32Z

example/points/Makefile.am

@@ -7,12 +7,19 @@ if P4EST_ENABLE_BUILD_2D
 bin_PROGRAMS += example/points/p4est_points
 example_points_p4est_points_SOURCES = example/points/points2.c

+bin_PROGRAMS += example/points/p4est_generate_points


Thanks; what about renaming to p4est_points_generate?

ok will do.

cburstedde · 2023-07-04T06:55:13Z

example/points/generate_points.c

+  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+*/
+
+#ifdef THREED


This macro THREED is unusual in all of p4est. Users might think that this is a convention they should copy in their own code. Is it possible to rely on P4_TO_P8 exclusively, like in the rest of p4est?

ok, I will turn generate_points.c into generate_points2.c and generate_points3.cand define P4_TO_P8 in the later.

cburstedde · 2023-07-04T06:56:22Z

example/points/Makefile.am

 LINT_CSOURCES += $(example_points_p4est_points_SOURCES)
 endif

 if P4EST_ENABLE_BUILD_3D
 bin_PROGRAMS += example/points/p8est_points
 example_points_p8est_points_SOURCES = example/points/points3.c

+bin_PROGRAMS += example/points/p8est_generate_points
+example_points_p8est_generate_points_SOURCES = example/points/generate_points.c
+example_points_p8est_generate_points_CPPFLAGS = -DTHREED $(AM_CPPFLAGS)


See below comment; such a THREED macro is usually not necessary in how we write dimension-dependent code.

cburstedde · 2023-07-04T06:57:06Z

example/points/generate_points.c

+
+
+static void
+generate_points (const char *filename,


Will this file compile when MPIIO is not enabled (try --disable-mpiio)? This relates to libsc, where MPI_File_get_view etc. are defined with MPIIO, but have no correspondence without it.

ok, MPIIO is required. We can disable building this example when mpi-io is not available.

This seems fine for now. @tim-griesbach do you see a way to abstract the MPI I/O status away in the future?

We can abstract the MPI I/O status away using the functions sc_io_open, sc_io_write_at_all, sc_io_read_at_all and sc_io_close since these functions work also without MPI I/O or even without MPI.

However, in points2.c the number of points on reading is currently determined using the file size. Up to my understanding this does not work without MPI I/O in a general portable way by just relying on the C standard (cf. https://wiki.sei.cmu.edu/confluence/display/c/FIO19-C.+Do+not+use+fseek()+and+ftell()+to+compute+the+size+of+a+regular+file).

We can solve this problem by writing the number of points to the file. Assuming this adjustment we shall obtain code that produces equivalent files with and without MPI I/O.

If you want, I can adjust the code as described above.

cburstedde · 2023-07-09T10:35:17Z

Thanks; I've merged the prerequisites in libsc!

I'm wondering whether the example itself may survive without MPIIO, just not the file generation -- is there a way?

cburstedde · 2023-07-12T08:10:11Z

This sounds all good. If points works generally with a change to the file format and using #ifdef P4EST_ENABLE_MPIIO and/or sc_io wrappers, please go ahead!

tim-griesbach · 2023-07-13T12:15:39Z

This sounds all good. If points works generally with a change to the file format and using #ifdef P4EST_ENABLE_MPIIO and/or sc_io wrappers, please go ahead!

I pushed changes to use the sc_io wrappers and to write/read the number of global points to/from the file.

During testing my changes I observed different behavior of sc_io_open in the create mode depending on the MPI IO status. These differences come from the difference between fopen with the mode 'w' that truncates the file to length zero and MPI_File_open that just overwrites an existing file in the create mode.

Therefore, I adjusted sc_io_open such that it also truncates the file to length zero in the case of the creation mode. The changes are in the PR cburstedde/libsc#130.

cburstedde · 2023-07-15T10:15:30Z

Would it be possible to rebase this branch on the latest develop without touching sc in the history?

pkestene · 2023-07-15T10:24:24Z

Would it be possible to rebase this branch on the latest develop without touching sc in the history?

I think commit b571e91 should be modified not to update sc git submodule; then the rebase would be easier.

cburstedde · 2023-07-15T10:41:00Z

> Would it be possible to rebase this branch on the latest develop without touching sc in the history? I think commit b571e91 should be modified not to update sc git submodule; then the rebase would be easier.

Is it possible to fix this during the rebase? When basing on develop, the sc submodule will always be the same at 04ec24f6869c08a7b76c14ce7684e9952.

points2 and points3 were deactivate from cmake build system, with comments stating that there is points data file. Here we add an executable generate_points to create some example points data. - the difference is that now there only a single data file (not a data file per MPI process), - point data are written by executable generate_point2/generate_point3 and read by points2/points3 using MPI-IO, so this branch need an upto data version of libsc (at least branch update-mpiio) - by using mpi-io this executable is more flexible, we can change the number of mpi process easily (no need to regenerate input data points) Example of run: - mpirun -np 4 ./generate_points2 unit 8192 data2d - mpirun -np 4 ./points2 unit 10 10 data2d

- rename and split generate_points.c into generate_points2.c and generate_points3.c - make sure this example is built only if mpi-io is enabled

pkestene · 2023-10-16T13:51:59Z

Hi @cburstedde
FYI, i've just rebase this onto latest develop and made simple adjustment to make it work again. I think it is in good shape now.

cburstedde · 2023-10-18T06:09:30Z

Hi @cburstedde FYI, i've just rebase this onto latest develop and made simple adjustment to make it work again. I think it is in good shape now.

Thanks so much, I'll take a look!

cburstedde · 2023-10-18T14:03:19Z

Thanks! This is all very nice now. You gave me some motivation to write some outstanding documentation as well, I hope you don't mind I pushed it onto your PR. I've also used the p4est partition formulas for reference, even though your formulas work just fine as well!

I see that the generation of points is independent of the connectivity. I'm wondering whether we should do something about that, but otoh we may all have better things to do right now. I'm ready to merge as is.

cburstedde · 2023-10-18T14:10:39Z

A note to @tim-griesbach: This example nicely demonstrates use of some sc_io functions. How hard would it be to introduce proper error handling, to make this example instructive and representative of safe I/O code in p4est?

tim-griesbach · 2023-10-18T17:40:32Z

A note to @tim-griesbach: This example nicely demonstrates use of some sc_io functions. How hard would it be to introduce proper error handling, to make this example instructive and representative of safe I/O code in p4est?

It should be easy to add in the case of an I/O error the conversion of the error code to an error string using sc_MPI_Error_class. This works independently of the availability of MPI (I/O) since the sc_io functions internally translate the error codes to MPI error classes or wrappers of these MPI error classes that work without MPI (I/O) and sc_MPI_Error_string also works without MPI I/O.

Then we could print the error string to inform the user about the specific error. If an I/O error occurs we would still abort after printing the error string.

cburstedde · 2023-10-19T08:13:16Z

Thanks all, I've updated #211 with the current status and ammerging now.

cburstedde added the enhancement label Jul 4, 2023

cburstedde reviewed Jul 4, 2023

View reviewed changes

tim-griesbach mentioned this pull request Jul 13, 2023

Make sc_io_open independent of the MPI IO status cburstedde/libsc#130

Merged

pkestene force-pushed the update-points-example branch 4 times, most recently from a809c10 to 2980811 Compare July 15, 2023 19:30

pkestene changed the base branch from master to develop July 25, 2023 11:51

pkestene and others added 13 commits October 16, 2023 14:39

Add missing mesh3 example.

cbf40b8

Update autotools to build helper exe to generate points data.

6bc5020

Addressing github MR comments.

de3cae0

- rename and split generate_points.c into generate_points2.c and generate_points3.c - make sure this example is built only if mpi-io is enabled

branch update-mpiio of libsc now defines SC_ENABLE_MPIIO, so use it.

3676116

example-points: Start adjustement of MPI IO status agnostic IO

7a792df

example-points: Broadcast number of points after reading

357e260

example-points: Remove unused variables

ce3dc48

example-points: Remove comments from makefile

eeb7257

example-points: Build without MPI IO with CMake

07b402d

Fixing unused var and warnings.

081a901

Fix comparison

1709f9b

Fix comparison

3ebb824

pkestene added 2 commits October 16, 2023 15:48

Fix cmake after rebase.

efffbc4

Update readme; swap partition and balance.

83f068d

pkestene force-pushed the update-points-example branch from 2980811 to 83f068d Compare October 16, 2023 13:49

cburstedde added 5 commits October 18, 2023 15:06

Example points: indent/whitespace

61c896e

Example points: use standard partition formula

dc380f1

Example points: work on a documentation page

c5d4b98

Example points: use standard partition formula

76bc78e

p4est_points: remove outdated disclamer on sc_sort

1367274

cburstedde mentioned this pull request Oct 19, 2023

points example lacking point data #211

Open

cburstedde merged commit 4631ce3 into cburstedde:develop Oct 19, 2023
36 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update points example #212

Update points example #212

pkestene commented Apr 30, 2023

cburstedde Jul 4, 2023

pkestene Jul 4, 2023

cburstedde Jul 4, 2023

pkestene Jul 4, 2023

cburstedde Jul 4, 2023

cburstedde Jul 4, 2023

pkestene Jul 4, 2023

cburstedde Jul 9, 2023

tim-griesbach Jul 10, 2023 •

edited

Loading

tim-griesbach Jul 10, 2023

cburstedde commented Jul 9, 2023

cburstedde commented Jul 12, 2023

tim-griesbach commented Jul 13, 2023 •

edited

Loading

cburstedde commented Jul 15, 2023

pkestene commented Jul 15, 2023

cburstedde commented Jul 15, 2023 via email

pkestene commented Oct 16, 2023

cburstedde commented Oct 18, 2023

cburstedde commented Oct 18, 2023

cburstedde commented Oct 18, 2023

tim-griesbach commented Oct 18, 2023 •

edited

Loading

cburstedde commented Oct 19, 2023

Update points example #212

Update points example #212

Conversation

pkestene commented Apr 30, 2023

Update points example

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

tim-griesbach Jul 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cburstedde commented Jul 9, 2023

cburstedde commented Jul 12, 2023

tim-griesbach commented Jul 13, 2023 • edited Loading

cburstedde commented Jul 15, 2023

pkestene commented Jul 15, 2023

cburstedde commented Jul 15, 2023 via email

pkestene commented Oct 16, 2023

cburstedde commented Oct 18, 2023

cburstedde commented Oct 18, 2023

cburstedde commented Oct 18, 2023

tim-griesbach commented Oct 18, 2023 • edited Loading

cburstedde commented Oct 19, 2023

tim-griesbach Jul 10, 2023 •

edited

Loading

tim-griesbach commented Jul 13, 2023 •

edited

Loading

tim-griesbach commented Oct 18, 2023 •

edited

Loading