Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prelim GDAL enabled PIO #1980

Closed
wants to merge 1 commit into from
Closed

Prelim GDAL enabled PIO #1980

wants to merge 1 commit into from

Conversation

msl3v
Copy link
Collaborator

@msl3v msl3v commented Jan 4, 2024

Created for diagnosis of shapefile read issue.

MSL

@jedwards4b jedwards4b marked this pull request as draft January 4, 2024 16:43
Copy link
Contributor

@jedwards4b jedwards4b left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like your branch is missing some history and you are trying to bring back a lot of old code - please clean that up and only bring in your intended changes. It also looks like you are trying to run the interface to gdal on all io tasks when it should only be run on io_root. When you have made these changes I'll take another look.

@@ -9,8 +9,8 @@ project (PIOC C)
if (CMAKE_BUILD_TYPE)
define_property(
SOURCE
PROPERTY COMPILE_FLAGS
INHERITED
PROPERTY COMPILE_FLAGS
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid adding unneeded whitespace.

@@ -1,5 +1,5 @@
/**
* c.f.: https://raw.githubusercontent.com/rabauke/mpl/main/examples/parallel_sort_mpi.c
* c.f.: https://raw.githubusercontent.com/rabauke/mpl/master/examples/parallel_sort_mpi.c
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for correcting.

int ierr;
if ((mpierr = MPI_Comm_rank(comm, &rank)))
check_mpi(NULL, NULL, mpierr, __FILE__, __LINE__);
check_mpi(NULL, NULL, mpierr2, __FILE__, __LINE__);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is wrong, mpierr is an input to check_mpi.

@@ -17,16 +17,9 @@
#include <uthash.h>

#include <netcdf.h>
#include <netcdf_meta.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this removed?


#include <gdal.h>
//#include <ogr_api.h>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the commented include?

(*pio_type)[v] = (int)PIO_STRING;
(*pio_type_size)[v] = -1;
break;
// This needs to be done. How do we deal with timestamps etc in GDAL vector fields?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we treat them as separate variables?

{
if (!ios->ioproc)
{
int msg = PIO_MSG_INQ_VARID;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You will need to define unique msg id's for your functions, this is part of the async support.

switch (file->iotype)
{
case PIO_IOTYPE_GDAL:
// if (ios->io_rank == 0)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this commented out? If your IO is limited to a serial interface you only want to open the file on the root io task.

return pio_err(NULL, NULL, ierr, __FILE__, __LINE__);

GDALDatasetH *hDS = file->hDS;
OGRLayerH hLayer = OGR_DS_GetLayer( hDS, 0 );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this also be limited to io_root?

case PIO_IOTYPE_GDAL:
PLOG((2, "Calling GDALCreate io_comm = %d mode = %d fh = %d",
ios->io_comm, mode, file->fh));

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that these calls should be restricted to io_root.

@jedwards4b
Copy link
Contributor

Superseded by #1981

@jedwards4b jedwards4b closed this Jan 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants