search

MPAS-Dev / MPAS-Tools

MPAS Tools Repository
Other
37 stars 63 forks source link

Y-periodicity broken in mesh_converter #289

Open jamilgafur opened 4 years ago

jamilgafur commented 4 years ago

If you use the default planar_hex inside the init file it the boundary conditions work as intended (overflow to the next region as shown below)

KPP_tracer1

However, when it is passed through the mesh_convertor, it seems to get caught in the region (as shown below)

KPP_tracer1

jamilgafur commented 4 years ago

would any of you have any recommendations on why this is the case?

@vanroekel @pwolfram @xylar

Thank you for your time

xylar commented 4 years ago

@jamilgafur, could you provide some more detailed instructions on how to reproduce this issue? Probably running your test case might be too much but at least the process you went through to generate each mesh.

jamilgafur commented 4 years ago

The working test branch can be found here.

If you navigate to the config_init file here

we have the following code snippet: 

<step executable="MpasMeshConverter.x"> 
        <argument flag="">mesh.nc</argument>
        <argument flag="">mesh2.nc</argument>
</step>

In order to get the code to run we need to add a random second argument (i.e mesh2.nc) instead of the default:

<step executable="MpasMeshConverter.x">
                        <argument flag="">base_mesh.nc</argument>
                        <argument flag="">mesh.nc</argument>
</step>
xylar commented 4 years ago

@jamilgafur, I'm trying out your test case. I think you may be using a pretty old version of COMPASS that still points to a location for the mesh converter with the mesh_converter option. Could you rebase your work onto MPAS-Dev/MPAS-Model/ocean/develop and make sure you are using a COMPASS conda environment that includes the mesh converter from the mpas_tools conda package?

See https://github.com/MPAS-Dev/MPAS-Model/blob/ocean/develop/testing_and_setup/compass/README_ocean.md for more information and write back if you need some assistance.

I'll be off to Ocean Sciences soon so this may have to wait until after.

xylar commented 4 years ago

@jamilgafur, I doubt it's going to be fixed, but we realized there were some remaining issues with the x_period and the sphere_radius in the mesh conversion tools (including the mesh converter). See #296. I doubt this will fix the issue you found but it would be worth using compass_0.1.1 as soon as I make it and re-testing your test case. I will keep you posted.

xylar commented 3 years ago

@qingli411, I think this is the same issue you mentioned to me in email.

I don't think we have a good solution for it other than not running the mesh through the mesh converter, which does seem to work at least in cases we've tried.

xylar commented 3 years ago

We would really appreciate a fix for this but we haven't had time to figure out the cause, let alone fix it.

qingli411 commented 3 years ago

Thanks, @xylar! OK, now I remember it...

xylar commented 3 years ago

@matthewhoffman, @alicebarthel indicated to me that you aren't aware of this long-standing issue with the mesh converter and periodic meshes. Please take a look and let me know if you have an interest in investigating this. It's been on my to-do list forever.

matthewhoffman commented 3 years ago

@xylar , thanks for directing me to this. I had not seen it (or did and then forgot). That obviously changes a lot of the suggestions I told @alicebarthel when we talked about the issues she was seeing. I'm interested in looking at this, but I'm not sure how soon I might get to it.

matthewhoffman commented 3 years ago

@xylar , do you think this fix may be as simple as changing the data type to double for the other attributes as in #296 ? Or is that not clear yet?

xylar commented 3 years ago

@matthewhoffman, I don't think it's clear if it's something that simple but it would certainly be nice if it were. Part of the problem has been that not running periodic meshes through the mesh converter seems to be a fix without any side effects, so it's been hard to justify putting time toward this fix so far. But the fact that it's deteriorated trust in the mesh converter in other cases certainly starts to make it seem more important to fix.

xylar commented 3 years ago

I looked at https://github.com/MPAS-Dev/MPAS-Tools/pull/296 again and it makes clear that it was at least intended to take care of remaining variables that were being read with an incorrect type in the converter. The original PR that missed some variables was https://github.com/MPAS-Dev/MPAS-Tools/pull/279. So I don't think there's any indication that these type issues are related to this issue, and we have seen related problems with periodic meshes since https://github.com/MPAS-Dev/MPAS-Tools/pull/296.