Map/connectivity separation

[hannesbrandt]

I saw that my recent PR https://github.com/ForestClaw/forestclaw/pull/271 on map/connectivity separation just got merged and wanted to ask about the status for the remaining new_conn_map occurences. Since @donnaaboise mentioned that she worked on this too, I want to make sure that I do not interfere with your progress on this topic. I am happy to help with any future work regarding this subject.

[donnaaboise]

No problem - I'll take a look at what you have. I was making a few more cosmetic changes that I can take care of. I am going to work on this over the next few days

[donnaaboise]

It seems that there are new functions added. Before, we had :

) {
    case 0:
        conn = p4est_connectivity_new_cubed();
        cont = fclaw2d_map_new_cubedsphere(fclaw_opt->scale,rotate);

followed by

fclaw2d_domain_t *domain = 
        fclaw2d_domain_new_conn(glob->mpicomm, fclaw_opt->minlevel, conn);

Looking at the latest by @hannesbrandt , this has been changed to :

domain  = fclaw2d_domain_new_cubedsphere (glob->mpicomm, fclaw_opt->minlevel);
cont = fclaw2d_map_new_cubedsphere (fclaw_opt->scale, rotate);

Is there a reason new domains are now "connectivity specific"? It does reduce the number of calls, and doesn't expose the connectivity to the user. Anything else?

[cburstedde]

It seems that there are new functions added. Before, we had :

) {
    case 0:
        conn = p4est_connectivity_new_cubed();
        cont = fclaw2d_map_new_cubedsphere(fclaw_opt->scale,rotate);

followed by

fclaw2d_domain_t *domain = 
        fclaw2d_domain_new_conn(glob->mpicomm, fclaw_opt->minlevel, conn);

Looking at the latest by @hannesbrandt , this has been changed to :

domain  = fclaw2d_domain_new_cubedsphere (glob->mpicomm, fclaw_opt->minlevel);
cont = fclaw2d_map_new_cubedsphere (fclaw_opt->scale, rotate);

The previous call fclaw2d_domain_new_conn is still valid and will remain. It is the most general way to construct a domain. Please feel free to use that one if you prefer it. It allows, for example, to switch an example on the connectivities and program the domain construction only once.

Is there a reason new domains are now "connectivity specific"? It does reduce the number of calls, and doesn't expose the connectivity to the user. Anything else?

You're right, this is just a convenience function for common cases, and the user does not see any dependence on p4est functions.

[tim-griesbach]

In the PR https://github.com/ForestClaw/forestclaw/pull/280 @scottaiton separated mappings in the Geoclaw, ThunderEgg, and CudaClaw examples. Since we plan to eventually remove the dependency of fclaw2d_convenience.{c,h} on the mappings I checked if we can already remove the map dependency in fclaw2d_convenience.{c,h} .

After adjusting applications/clawpack/advection/2d/swirl_rays/swirl.cpp in the same way as used in the PR https://github.com/ForestClaw/forestclaw/pull/280 the code compiled with the configure flags --enable-paper and --enable-clawpack. However, there are still calls of fclaw2d_domain_new_conn_map in the code, which are not compiled in this configuration. Furthermore, it seems to me that these files are not compiled for any build configuration because the respective files are commented out in the build, e.g. in applications/geoclaw/geoclaw.apps.

A special case of a not compiled file is applications/clawpack/advection/2d/filament_swirl/filament/filament.cpp since this file seems to duplicate the functionality of the file applications/clawpack/advection/2d/filament_swirl/filament.cpp.

Is the map separation still work in progress or is the plan to leave the remaining files as they are now?