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

Issue264 netcdf strings #330

Merged
merged 2 commits into from
Jun 14, 2023
Merged

Issue264 netcdf strings #330

merged 2 commits into from
Jun 14, 2023

Conversation

draenog
Copy link
Collaborator

@draenog draenog commented Jun 12, 2023

Fixes #264

Change description

The PR replaces NetCDF4 string variables with attributes as writing such variables is not supported in parallel mode. For consistency it also writes all other members of configuration group as attributes.

  • formatted time is written as attribute to time variable
  • write all members of configuration group as attributes for consistency

@draenog draenog requested a review from timspainNERSC June 12, 2023 09:28
Copy link
Collaborator

@timspainNERSC timspainNERSC left a comment

Choose a reason for hiding this comment

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

Looks good apart from the possible unused double value.

// All other configuration data
netCDF::NcGroup configGroup = metaGroup.addGroup(configurationNode());

for (auto entry : metadata.m_config) {
switch (entry.second.index()) {
case (CONFIGMAP_DOUBLE): {
netCDF::NcVar dblVar = configGroup.addVar(entry.first, netCDF::ncDouble);
dblVar.putVar(std::get_if<double>(&entry.second));
double value = 1.5;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is doing anything? Unless it is really subtle.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for spotting it. It was leftover from my debugging. I've pushed the fixed version (debc4bf).

draenog added 2 commits June 12, 2023 13:33
Parallel netCDF4 does not support writing string variables in parallel
mode so the formatted time needs to be recorded as an attribute.
@draenog draenog force-pushed the issue264_netcdf_strings branch from a2ef2eb to debc4bf Compare June 12, 2023 12:37
@draenog draenog merged commit 10df4e8 into develop Jun 14, 2023
@draenog draenog added the ICCS Tasks or reviews for the ICCS team label Jun 14, 2023
TomMelt added a commit that referenced this pull request Dec 4, 2023
## Background 
This PR is part of #120, where the basis of strategy for MPI
parallelisation is described.

## Change description
As all thermodynamics operations are local to a grid cell all required
MPI communication, is handled by NetCDF4 library. Therefore the only
required steps are:

- [x] Initialize and finalize MPI stack
- [x] Read decomposition metadata on each rank
- [x] Read and write the necessary part of the grid on each rank
(depends on #330)
- [x] Tests for parallel I/O 

**NOTE** PR #432 should be merged into this branch before it is merged
into develop

To run `run_simple_example.sh` you will need to generate the following
netcdf file

```
$ ncdump partition.nc 
netcdf partition {
dimensions:
        P = 1 ;
        L = 1 ;
        globalX = 30 ;
        globalY = 30 ;

group: bounding_boxes {
  variables:
        int global_x(P) ;
        int global_y(P) ;
        int local_extent_x(P) ;
        int local_extent_y(P) ;
  data:

   global_x = 0 ;

   global_y = 0 ;

   local_extent_x = 30 ;

   local_extent_y = 30 ;
  } // group bounding_boxes
}

```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ICCS Tasks or reviews for the ICCS team
Projects
Development

Successfully merging this pull request may close these issues.

2 participants