Add mesh connectivity in atlas by travissluka · Pull Request #983 · JCSDA-internal/soca

travissluka · 2023-12-06T17:06:44Z

Description

"evaporates the point cloud functionspace" 🌧️

closes Update ATLAS interface stuff #975

Builds a proper atlas functionspace with mesh connectivity.

Geometry constructor has been changed to pull the mesh information from the Fortran side, then pass the constructed functionspace back to Fortran
remove some stuff that is no longer needed (e.g. toFieldSetAD(), passing lonlat from Fortran to C++, the weird hacks that were needed to avoid duplicate halo points)
added an option to save gmsh data when the grid is generated so that it can be viewed with gmsh (see images below, and gridgen.yml)
a bunch of other minor things that were needed because of the first 2 bullet points
references change because the interpolation is slightly different

Testing

note, spack stack 1.5.1 is required for these branches to work.

Here is what the grid looks like with the 5deg grid on 16 PEs

And if you're curious how weird the tripolar fold looks, here is one PE with halos

ctests pass
test with regional HAT10 on orion
test with global 1/4

Dependencies

build-group=https://github.com/JCSDA-internal/oops/pull/2465
build-group=https://github.com/JCSDA-internal/saber/pull/729

kbhargava · 2023-12-06T22:05:11Z

I was able to build this after the soca-science/#625 on orion that updates spack-stack version. All the ctests pass for soca. I am testing with HAT10 now, will update soon.

fmahebert

Mine is not an expert review, but these code changes look as expected to me

src/soca/Geometry/Geometry.cc

src/soca/Geometry/soca_geom_mod.F90

travissluka · 2023-12-07T16:38:56Z

@hga007 FYI, in case any of this is useful to you. Yours should be more straightforward since you don't have the northern tripolar fold or cyclic E/W boundaries to deal with. I have code here that does extra checking to see which duplicate halo points need to be removed before being passed to atlas... but you shouldn't have any duplicates to worry about.

hga007 · 2023-12-07T16:44:09Z

@travissluka, Thank you! Indeed, it will be easier. I looked at your code and think it will be easier if I follow your approach. You did an excellent work implementing this feature. Cheers.

travissluka · 2023-12-07T23:21:18Z

note, tests pass, except for clang. not sure why, I don't have clang.... ignore that one for now.

travissluka · 2023-12-08T17:39:00Z

note to self, I disabled the FPE trapping on the diffusion dirac test. I'm not entirely sure why that's failing on clang only (see https://cdash.jcsda.org/test/186729). Odds are, like the other FPE issues, its something weird going on in a region of the halo that I don't care about, or on land. I'm not opening an issue because it's already on my todo list to someday go through and fix and remove as many NOTRAPFPE directives as possible in our ctests... someday

kbhargava · 2023-12-08T19:34:53Z

The regional experiment fails with the following error

Assertion failed: Assertion [part(209) < proc.size()] failed
part(209) = 1079164896
proc.size() = 480 in build_nodes_remote_idx, line 311 of /work/noaa/epic/role-epic/spack-stack/orion/spac+ k-stack-1.5.1/cache/build_stage/spack-stage-ecmwf-atlas-0.35.0-mkl256cnvwvp6xcrywhjqsnofzkarieb/spack-s+ rc/src/atlas/mesh/actions/BuildParallelFields.cc
Assertion failed: Assertion [part(15) < proc.size()] failed
part(15) = 1062232653
proc.size() = 480 in build_nodes_remote_idx, line 311 of /work/noaa/epic/role-epic/spack-stack/orion/spac+ k-stack-1.5.1/cache/build_stage/spack-stage-ecmwf-atlas-0.35.0-mkl256cnvwvp6xcrywhjqsnofzkarieb/spack-s+ rc/src/atlas/mesh/actions/BuildParallelFields.cc
[Orion-19-69:335639:0:335639] Caught signal 11 (Segmentation fault: address not mapped to object at addre+ ss 0xfffffffebe91d174)

travissluka · 2023-12-08T19:59:56Z

The regional experiment fails with the following error

Assertion failed: Assertion [part(209) < proc.size()] failed
part(209) = 1079164896
proc.size() = 480 in build_nodes_remote_idx, line 311 of /work/noaa/epic/role-epic/spack-stack/orion/spac+ k-stack-1.5.1/cache/build_stage/spack-stage-ecmwf-atlas-0.35.0-mkl256cnvwvp6xcrywhjqsnofzkarieb/spack-s+ rc/src/atlas/mesh/actions/BuildParallelFields.cc
Assertion failed: Assertion [part(15) < proc.size()] failed
part(15) = 1062232653
proc.size() = 480 in build_nodes_remote_idx, line 311 of /work/noaa/epic/role-epic/spack-stack/orion/spac+ k-stack-1.5.1/cache/build_stage/spack-stage-ecmwf-atlas-0.35.0-mkl256cnvwvp6xcrywhjqsnofzkarieb/spack-s+ rc/src/atlas/mesh/actions/BuildParallelFields.cc
[Orion-19-69:335639:0:335639] Caught signal 11 (Segmentation fault: address not mapped to object at addre+ ss 0xfffffffebe91d174)

That's weird, me thinking "eh, it should work, I don't need to test it with regional!" is usually never wrong!!

travissluka · 2023-12-08T20:03:12Z

@fmahebert I could look at your code, but it's probably faster for me to ask you: for the regional FV3, with the "halo" points that aren't owned by anyone because they are at the boundary of the regional domain, do you just assign them to the PE for the patch that is sitting closest to those "boundary halo-ish" points?

fmahebert · 2023-12-08T20:08:44Z

test/CMakeLists.txt

               TEST_DEPENDS test_soca_gridgen
                            test_soca_static_socaerror_init)

+# NOTE the following test throws a FPE only on clang


In the past I've seen clang-specific FPEs arising from clang's choice to optimize by speculatively evaluating possible branches, ignoring the fact that evaluating floating-point operations can have side effects like FPEs. As an example of what I mean, you could hide a division under if (denominator != 0) but clang might evaluate the branch anyway just in case it can use that early evaluation to optimize the code.

Could be what's happening here.

One workaround, if this is the problem and after it is localized, is to insert something the optimizer can't optimize into the if block, like an assembly-level instruction.

I say all this not as a change request but to share the info...

ah, thanks for the info, that's good to know. That sounds like a likely cause, I believe the error is happening inside a gpnorm function where divide by 0 is possible so there should be an if checking for that.

Someday I'll get clang working on my linux machine to debug things like this.

fmahebert · 2023-12-08T20:18:00Z

@fmahebert I could look at your code, but it's probably faster for me to ask you: for the regional FV3, with the "halo" points that aren't owned by anyone because they are at the boundary of the regional domain, do you just assign them to the PE for the patch that is sitting closest to those "boundary halo-ish" points?

@travissluka that's correct. My rationale was that if we later want to apply a proper boundary condition, that would likely be populated into the fv3 halos on that PE anyway.

travissluka · 2023-12-08T21:19:39Z

@kbhargava it should work for hat10 now. I only ran a check on the gridgen, not a full cycle. But I have changed the logic so that the nodes/cells representing the halo outside the domain to the north and east are removed. (note to self, that in the future we'll want to change that logic when/if we remove the "virtual land" that surrounds our domain)

here is the mesh with HAT10 on 16 PEs

and as a sanity check, a PE in the middle of the domain has halos on all sides:

but the PE on the NE corner does not have halos above and to the right

guillaumevernieres

Did you test at "high-res" @travissluka or @kbhargava ? Did you notice a change in the time it takes to run the geometry constructor?
Thanks for the pretty figures ...

kbhargava · 2023-12-11T15:30:17Z

prep.soca step passes now with the regional case. 🎉

kbhargava

Thanks Travis

travissluka · 2023-12-11T17:04:05Z

Did you test at "high-res" @travissluka or @kbhargava ? Did you notice a change in the time it takes to run the geometry constructor? Thanks for the pretty figures ...

@guillaumevernieres you doubt my mental ctests??? Fine, let me test with 1/4 deg global.

oh wait...

Node uid: 2918430278073000000   0 xy(-999.0000000000,-999.0000000000) has already been added as node 0 (-999.0000000000,-999.0000000000)
Node uid: 2918430278073000000   0 xy(-999.0000000000,-999.0000000000) has already been added as node 0 (-999.0000000000,-999.0000000000)
Node uid: 2918430278073000000   0 xy(-999.0000000000,-999.0000000000) has already been added as node 0 (-999.0000000000,-999.0000000000)
Node uid: 2918430278073000000   0 xy(-999.0000000000,-999.0000000000) has already been added as node 0 (-999.0000000000,-999.0000000000)
backtrace [1] stack has 13 addresses
(/home/tsluka/work/spack-stack/envs/skylab.1.5.1_gcc.12.3.0/install/gcc/12.3.0/eckit-1.24.4-tk6ui5d/lib/libeckit.so+eckit::BackTrace::dump[abi:cxx11]())0x1db 
(/home/tsluka/work/spack-stack/envs/skylab.1.5.1_gcc.12.3.0/install/gcc/12.3.0/eckit-1.24.4-tk6ui5d/lib/libeckit.so+eckit::Exception::Exception(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, eckit::CodeLocation const&, bool))0xb3 
(/home/tsluka/work/spack-stack/envs/skylab.1.5.1_gcc.12.3.0/install/gcc/12.3.0/ecmwf-atlas-0.35.0-tez5dnr/lib/libatlas.so.0.35+atlas::throw_Exception(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, eckit::CodeLocation const&))0x29 
(/home/tsluka/work/spack-stack/envs/skylab.1.5.1_gcc.12.3.0/install/gcc/12.3.0/ecmwf-atlas-0.35.0-tez5dnr/lib/libatlas.so.0.35+atlas::mesh::actions::build_lookup_uid2node(atlas::Mesh&, std::unordered_map<long, int, std::hash<long>, std::equal_to<long>, std::allocator<std::pair<long const, int> > >&))0xdca 
(/home/tsluka/work/spack-stack/envs/skylab.1.5.1_gcc.12.3.0/install/gcc/12.3.0/ecmwf-atlas-0.35.0-tez5dnr/lib/libatlas.so.0.35+atlas::mesh::actions::increase_halo_interior(atlas::mesh::actions::BuildHaloHelper&))0x3d8 
(/home/tsluka/work/spack-stack/envs/skylab.1.5.1_gcc.12.3.0/install/gcc/12.3.0/ecmwf-atlas-0.35.0-tez5dnr/lib/libatlas.so.0.35+atlas::mesh::actions::BuildHalo::operator()(int))0x53b 
(/home/tsluka/work/soca/build_gcc11/bin/../lib/libsoca.so+soca::Geometry::Geometry(eckit::Configuration const&, eckit::mpi::Comm const&, bool))0xfbe 
(./soca_gridgen.x+)0x268c 
(/home/tsluka/work/soca/build_gcc11/bin/../lib/liboops.so+oops::Run::execute(oops::Application const&, eckit::mpi::Comm const&))0x2c2 
(./soca_gridgen.x+main)0x5a 
(/lib/x86_64-linux-gnu/libc.so.6+)0x280d0 
(/lib/x86_64-linux-gnu/libc.so.6+__libc_start_main)0x89 
(./soca_gridgen.x+)0x2365

travissluka · 2023-12-11T17:48:41Z

@guillaumevernieres @kbhargava never mind, false alarm!

I was trying to test 1/4 deg but my MOM_input file did not have TRIPOLAR_N=True so bad things were happening. We'll have to update the static files for skylab because I don't think MOM_input has that set.

I only tested the grid generation, I did not test a full cycle (I leave that as an exercise for the user!). Here is 1/4 deg with 32 PEs

and the last PE along the tripolar fold showing halos

and @guillaumevernieres there is no noticeable difference in run time, creating the atlas mesh is fast.

fmahebert

Thanks @travissluka!

fmahebert · 2023-12-12T16:57:14Z

The OOPS PR has been merged; this PR can be merged as well.

(Unless if the clang CI needs more attention?)

travissluka · 2023-12-12T17:14:50Z

The OOPS PR has been merged; this PR can be merged as well.

(Unless if the clang CI needs more attention?)

@fmahebert the clang CI was apparently breaking with NaNs before this PR... i just never noticed it. Deal with that later.

travissluka added 15 commits November 29, 2023 12:09

remove function space (without halo)

93283df

wip

bd3b918

WIP

ee8e591

mesh build working

f8ec424

remove toFieldsetAD

56ba387

fix bug when only using 1 PE

fcea4a2

fix node removal logic again. all edge cases SHOULD be handled now

e33dc3e

working?!

5cd0530

Merge remote-tracking branch 'origin/develop' into feature/atlas

0a663b0

make saving gmsh optional

8b302ae

small cleanup

cfe8b72

small cleanup

78cb9ab

cleanup

a15ea87

cleanup, again

24004aa

cleanup, aaagain

caa4b9f

travissluka added the waiting for another PR waiting on another pull request to be merged first label Dec 6, 2023

travissluka self-assigned this Dec 6, 2023

travissluka requested review from fmahebert, guillaumevernieres and kbhargava December 6, 2023 17:07

fmahebert reviewed Dec 7, 2023

View reviewed changes

src/soca/Geometry/Geometry.cc Show resolved Hide resolved

src/soca/Geometry/soca_geom_mod.F90 Outdated Show resolved Hide resolved

travissluka and others added 2 commits December 7, 2023 11:57

Merge branch 'develop' into feature/atlas

f1f98d5

update soca2cice ref

3e8e3f5

disable FPE trap on dirac diffusion test

f558639

fmahebert reviewed Dec 8, 2023

View reviewed changes

travissluka and others added 2 commits December 8, 2023 14:13

fix for regional domain

cd32088

Merge branch 'develop' into feature/atlas

a433b76

guillaumevernieres approved these changes Dec 10, 2023

View reviewed changes

kbhargava approved these changes Dec 11, 2023

View reviewed changes

fmahebert approved these changes Dec 11, 2023

View reviewed changes

travissluka merged commit 15eab96 into develop Dec 12, 2023

travissluka deleted the feature/atlas branch December 12, 2023 17:15

Conversation

travissluka commented Dec 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Dependencies

Uh oh!

kbhargava commented Dec 6, 2023

Uh oh!

fmahebert left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

travissluka commented Dec 7, 2023

Uh oh!

hga007 commented Dec 7, 2023

Uh oh!

travissluka commented Dec 7, 2023

Uh oh!

travissluka commented Dec 8, 2023

Uh oh!

kbhargava commented Dec 8, 2023

Uh oh!

travissluka commented Dec 8, 2023

Uh oh!

travissluka commented Dec 8, 2023

Uh oh!

fmahebert Dec 8, 2023

Choose a reason for hiding this comment

Uh oh!

travissluka Dec 8, 2023

Choose a reason for hiding this comment

Uh oh!

fmahebert commented Dec 8, 2023

Uh oh!

travissluka commented Dec 8, 2023

Uh oh!

guillaumevernieres left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kbhargava commented Dec 11, 2023

Uh oh!

kbhargava left a comment

Choose a reason for hiding this comment

Uh oh!

travissluka commented Dec 11, 2023

Uh oh!

travissluka commented Dec 11, 2023

Uh oh!

fmahebert left a comment

Choose a reason for hiding this comment

Uh oh!

fmahebert commented Dec 12, 2023

Uh oh!

travissluka commented Dec 12, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

travissluka commented Dec 6, 2023 •

edited

Loading

guillaumevernieres left a comment •

edited

Loading