MPAS-Dev / MPAS-Tools

MPAS Tools Repository
Other
37 stars 63 forks source link

Rework file I/O in mesh converters for thread-safety #437

Closed dengwirda closed 2 years ago

dengwirda commented 3 years ago

This PR reworks the way the C++ MpasMeshConverter.x and MpasCellCuller.x, as well as the MPAS-Tools conversion.py wrapper, handles file I/O to eliminate the need to change into local directories to account for hard-coded relative file names (e.g. graph.info).

Eliminating the os.getcwd() + os.chdir() pattern in conversion.py makes these routines thread-safe, allowing them to be called across parallel threads.

These changes write all output files generated by the mesh-converter/culler (e.g. graph.info, cellMapForward.txt, etc) into the directory defined by the output filename.

dengwirda commented 3 years ago

To run additional tests (attached):

cd conda_package
conda env create -y -n mpas_tools_dev --file dev-spec.txt
conda activate mpas_tools_dev
python -m pip install -e .
cd ..
cd mesh_tools/mesh_conversion_tools
mkdir build
cd build
cmake -DCMAKE_INSTALL_PREFIX=$CONDA_PREFIX -DCMAKE_BUILD_TYPE=Debug ..
make
make install
python test_sphere.py
python test_planar.py

test_mesh_converter.zip

xylar commented 3 years ago

@mgduda, @akturner, @matthewhoffman and @mark-petersen, this work is needed as part of working on supporting parallel tasks via Parsl. If we make meshes in parallel with threads, it turns out that we can't be doing chdir type operations because these aren't thread-safe in python.

xylar commented 3 years ago

@dengwirda, could you add to the PR description that the approach for modifying the C++ code is to use the directory of the output file also as the directory for the other files (e.g. graph.info)? I think that's missing.

xylar commented 3 years ago

@dengwirda, I tested this in compass on both my Linux laptop and Chrysalis. I needed to build the conda package (so that conda-forge compilers were used instead of system compilers) rather than following the instructions above, which used system compilers. When I did that, I got bit-for-bit identical results with the ocean nightly test suite for all tests.

I will approve as soon as the formatting issues are fixed.

xylar commented 3 years ago

@mgduda, I went ahead and fixed the white space since it seems like @dengwirda hasn't had time. Could you give it another look to make sure you're okay with the changes otherwise? I would like to do a new release of MPAS-Tools soon and it would be nice if this were included.

xylar commented 2 years ago

@mgduda, is this something you might have time to try out soon? If so, the timing would be good with other fixes we want to make for another MPAS-Tools release.

xylar commented 2 years ago

Thanks @mgduda!