mdolab / pysurf

pySurf provides geometric operations for triangulated surfaces.
Apache License 2.0
2 stars 4 forks source link

Reorganized into Python package with just TSurf #7

Closed sseraj closed 3 years ago

sseraj commented 3 years ago

Purpose

This is a major reorganization of the repo with the following key changes:

After this is merged, I will add pySurf to the Docker builds.

Type of change

Testing

Run the new tests with testflo.

Checklist

codecov[bot] commented 3 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@2af054f). Click here to learn what that means. The diff coverage is n/a.

:exclamation: Current head 1b93404 differs from pull request most recent head 5558ae8. Consider uploading reports for the commit 5558ae8 to get more accurate results Impacted file tree graph

@@            Coverage Diff            @@
##             master       #7   +/-   ##
=========================================
  Coverage          ?   46.97%           
=========================================
  Files             ?        5           
  Lines             ?     1816           
  Branches          ?        0           
=========================================
  Hits              ?      853           
  Misses            ?      963           
  Partials          ?        0           

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2af054f...5558ae8. Read the comment docs.

anilyil commented 3 years ago

This looks great; thank you for working on this. I have a few questions/comments:

  1. The imports in dvgeomulti need to be updated right? It should be very simple but just checking.
  2. Can we make a release before and after this PR? The unused python code was crucial for me to understand how to use the fortran routines, so still easily accessing the latest version of the code by checking out the earlier tag sounds easy enough.
  3. Let's say we want to recover the previous python functionality that Ney had. What are the changes we would need to make? Can we take the old python code, integrate it with the current version, and debug it w/o re-writing the entire thing?

I am mostly just curious about these, rather than expecting changes to address anything. The PR looks good, but just wanted to document a few things here so we can just read this PR rather than try to figure things out in the future from scratch.

sseraj commented 3 years ago
  1. The imports in dvgeomulti need to be updated right? It should be very simple but just checking.

Yes, I updated them to: from pysurf import intersectionAPI, curveSearchAPI, utilitiesAPI, tsurf_tools, adtAPI, tecplot_interface

  1. Can we make a release before and after this PR? The unused python code was crucial for me to understand how to use the fortran routines, so still easily accessing the latest version of the code by checking out the earlier tag sounds easy enough.

Yes, that is my plan. The current release is v1.1.0 and is up to date. I will release v1.2.0 after this is merged. Can you clarify which unused Python code you found useful? I kept almost all the TSurf related Python code.

  1. Let's say we want to recover the previous python functionality that Ney had. What are the changes we would need to make? Can we take the old python code, integrate it with the current version, and debug it w/o re-writing the entire thing?

For just the hyperbolic surface mesh generation, we would have to bring back the hypSurf Fortran and Python code. This would mainly be a matter of rearranging the old code in the new directories. I have never gotten any of the hypSurf examples or tests to run though, so getting it functional might require some effort.

For the full functionality of computing intersections and regenerating meshes inside the optimization loop, in addition to getting hypSurf working, we would have to debug the Manager class and airfoil_intersection function in tsurf_tools. This would be fairly involved and rewriting these might actually be an easier approach.

anilyil commented 3 years ago
  1. Sounds good.
  2. I don't remember exactly which python routines were useful. If you kept the tsurf stuff, it was most likely those.
  3. Okay, that sounds reasonable. I think one issue with the previous implementation was that it deviated from the "default" MACH API quite a bit. So if we can recover the surface mesh marching, I would have also suggested moving away from the manager stuff to an approach thats a bit more consistent with our geometry and solver interfaces.

Thanks again for this work!