Skip to content

Conversation

@xylar
Copy link
Collaborator

@xylar xylar commented Apr 16, 2020

This feature makes sure that the Black Sea is not included in MPAS meshes (as long as they use critical passages and critical land blockages, which high resolution meshes typically do not).

@xylar
Copy link
Collaborator Author

xylar commented Apr 16, 2020

@maltrud, @milenaveneziani, @jonbob and @mark-petersen, just take a look at this:
https://github.com/xylar/geometric_features/blob/add_black_sea_blockage/geometric_data/ocean/transect/Black_Sea_Blockage/transect.geojson
and let me know if you agree that this should be blocked off. Nothing else needed for your review for now.

@xylar xylar changed the title Add Black Sea Blockage Add Black Sea bockage Apr 16, 2020
@jonbob
Copy link

jonbob commented Apr 16, 2020

@xylar - does it also get rid of the Caspian?

@xylar
Copy link
Collaborator Author

xylar commented Apr 16, 2020

I don't see any way the Caspian Sea would ever connect to the open ocean. If it were through the Black Sea, then yes this would block it, too. Do you know of meshes where it wasn't culled?

@jonbob
Copy link

jonbob commented Apr 16, 2020

I think it's always been through the Black Sea, so this should take care of it. I just wasn't picturing it from the blockage transect

Copy link

@jonbob jonbob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good from visual inspection

Copy link
Collaborator

@mark-petersen mark-petersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, looks good. Thanks!

@xylar
Copy link
Collaborator Author

xylar commented Apr 19, 2020

@milenaveneziani and @maltrud, do you want to review this? It takes quite a bit of work to update geometric_features and compass so these changes make it into meshes, and I'd like to get started on that as soon as I can.

@xylar xylar requested a review from vanroekel April 19, 2020 08:50
@xylar
Copy link
Collaborator Author

xylar commented Apr 19, 2020

@vanroekel, would you also give it a look?

Copy link
Collaborator

@vanroekel vanroekel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes look good to me. Approved via visual inspection

@xylar
Copy link
Collaborator Author

xylar commented Apr 19, 2020

I did a run with the mesh MPAS-Dev/MPAS-Model#518 using a compass environment with this version of geometric features. I verified that the Black Sea blockage is being included along with the 4 other blockages and that the mesh doesn't have the Black Sea (though this is not because of the blockage in this case).

xylar added 2 commits April 19, 2020 22:00
This feature makes sure that the Black Sea is not included in
MPAS meshes.
@xylar xylar force-pushed the add_black_sea_blockage branch from 3f9757e to 4be46cd Compare April 19, 2020 20:01
@xylar xylar removed the request for review from maltrud April 20, 2020 08:42
@xylar xylar changed the title Add Black Sea bockage Add Black Sea blockage Apr 20, 2020
@xylar xylar merged commit fd5a72f into MPAS-Dev:master Apr 20, 2020
@xylar xylar deleted the add_black_sea_blockage branch April 20, 2020 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants