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

Y-periodicity broken in mesh_converter #289

Open
jamilgafur opened this issue Feb 13, 2020 · 13 comments
Open

Y-periodicity broken in mesh_converter #289

jamilgafur opened this issue Feb 13, 2020 · 13 comments
Assignees

Comments

@jamilgafur
Copy link

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
Copy link
Author

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

@vanroekel @pwolfram @xylar

Thank you for your time

@xylar
Copy link
Collaborator

xylar commented Feb 13, 2020

@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
Copy link
Author

jamilgafur commented Feb 13, 2020

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
Copy link
Collaborator

xylar commented Feb 13, 2020

@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
Copy link
Collaborator

xylar commented Mar 5, 2020

@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
Copy link
Collaborator

xylar commented Apr 13, 2021

@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
Copy link
Collaborator

xylar commented Apr 13, 2021

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

@qingli411
Copy link

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

@xylar
Copy link
Collaborator

xylar commented Aug 23, 2021

@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
Copy link
Member

@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
Copy link
Member

@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
Copy link
Collaborator

xylar commented Aug 23, 2021

@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
Copy link
Collaborator

xylar commented Aug 23, 2021

I looked at #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 #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 #296.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants