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

Issue425 Example runs should run #426

Merged
merged 6 commits into from
Oct 27, 2023
Merged

Conversation

timspainNERSC
Copy link
Collaborator

Example runs should run

Fixes #425

Change Description

Fixes the example runs launched from run_simple_example.sh and run_topaz128x128.sh so that they run and do not crash, at least not until the end of the run.


Test Description

The runs now execute mostly successfully.

@timspainNERSC timspainNERSC added the bug Something isn't working label Oct 26, 2023
@timspainNERSC timspainNERSC added this to the 2 Integration tests milestone Oct 26, 2023
@timspainNERSC timspainNERSC self-assigned this Oct 26, 2023
Copy link
Contributor

@MarionBWeinzierl MarionBWeinzierl left a comment

Choose a reason for hiding this comment

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

That ran for me with test_simple_example.sh and did not fail.

What do you mean with "runs mostly successful", though? The failing checks?

@timspainNERSC
Copy link
Collaborator Author

That ran for me with test_simple_example.sh and did not fail.

What do you mean with "runs mostly successful", though? The failing checks?

There was also a failing unit test, now fixed. But the other example topaz128x128 will complete its calculations and then crash with a segfault on exit as the netCDF files are not properly closed. The code to fix this is in PR #383, which is one of the higher priority of the waiting PRs. It is tied into the rest of the code in that PR in such a way that I'd rather not do the work to disentangle just the section that would prevent this crash.

Copy link
Contributor

@MarionBWeinzierl MarionBWeinzierl left a comment

Choose a reason for hiding this comment

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

LGTM, issue mentioned above will be fixed in other PR

@timspainNERSC timspainNERSC merged commit 1880c48 into develop Oct 27, 2023
3 checks passed
@TomMelt TomMelt deleted the issue425_exampleruns branch November 1, 2023 18:23
@TomMelt TomMelt mentioned this pull request Nov 9, 2023
4 tasks
TomMelt added a commit that referenced this pull request Dec 4, 2023
# Add Tests for MPI version of code

### Task List
- [x] add tests
- [x] rebase to develop
- [x] fix array ordering
- [x] ~update the readthedocs / readme with install instructions~ (moved
to #455)

---
# Change Description

I rebased @draenog's `mpi_local` and `mpi_local_tests` branches. This
introduced a bug due to array reordering that occurred as part of #426.
I have modified the MPI parts of `./core/src/RectGridIO.cpp` to reorder
array so that they now match the order in the serial version of the
code.

---
# Test Description

To run `testRectGrid_MPI` you will need the parallel netcdf config file
`partition_metadata_1.nc`. I don't want to put it in the github repo for
now. As discussed with @timspainNERSC , soon we will hopefully have an
ftp server for netcdf files.

You can generate the input using the following netcdf definition file
`partition_metadata_1.cdl`.
```
// partition_metadata_1.cdl
netcdf partition_metadata_1 {
dimensions:
	P = 1 ;
	L = 1 ;
	globalX = 25 ;
	globalY = 15 ;

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 = 25 ;

   local_extent_y = 15 ;
  } // group bounding_boxes
}
```

Then use `ncgen` to make the netcdf file `partition_metadata_1.nc`
```
ncgen partition_metadata_1.cdl -o partition_metadata_1.nc
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

Successfully merging this pull request may close these issues.

2 participants