Custom GrowthRatios and more Schedule variables #94

DavidAnderegg · 2024-09-04T13:41:31Z

Purpose

~~This PR adds the ability to set custom growth ratios on each layer. Additionally, more 'schedule' variables are introduced.~~

This PR adjusts how some options work. Apart from prescribing a single value, it is now possible to define a list with per-layer values. Additionally, it is possible to instruct pyHyp to interpolate the value linearly throughout the extrusion.

This PR removes the volSmoothSchedule variable as the same functionality is now possible with volSmoothIter.

Lastly, a new variable growthRatios is introduced to explicitly set the growth ratio. If it is set, the marchDist option is ignored. This variable works like the other changes introduce here.

Expected time until merged

1 week

Type of change

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (non-backwards-compatible fix or feature) - volSmoothSchedule is now obsolete and was removed
Code style update (formatting, renaming)
Refactoring (no functional changes, no API changes)
Documentation update
Maintenance update
Other (please describe)

Testing

I adjusted the naca0012_rans and corner test to include an 'interpolation' and a 'per-layer' case. I also added a test for nConstantStart and nConstantEnd options. And finally, i added a test for simpleOCart with xBounds and no nearfield mesh.

~~Here are the new ref-files for the new tests: schedule_ref_files.zip~~

Here are the new ref-files for the new tests: refFiles.zip

Checklist

I have run flake8 and black to make sure the Python code adheres to PEP-8 and is consistently formatted
I have formatted the Fortran code with fprettify or C/C++ code with clang-format as applicable
I have run unit and regression tests which pass locally with my changes
I have added new tests that prove my fix is effective or that my feature works
I have added necessary documentation

* Update setup.py * changed numpy to <1.24 * ran black * Bump patch version --------- Co-authored-by: sseraj <[email protected]>

* add config file for building docs * add newline --------- Co-authored-by: Marco Mangano <[email protected]>

* renamed Intel config file * renamed gfortran config file * updated azure script * added MPI if check in Intel config * minor cleanup * updated docs * fixed Intel env var

eirikurj · 2024-09-04T14:20:55Z

@DavidAnderegg thanks for the PR. Will try to take a look as soon as I can. In the meantime, can you address the failing tests.

DavidAnderegg · 2024-09-04T14:24:27Z

@eirikurj The tests fail because new the ref files have not been added. I attached a .zip in the description of this PR.

eirikurj · 2024-09-04T15:19:34Z

My bad, I only read the description in a rush and saw the test are failing. I can update the reference files. I skimmed the PR content, and it looks good, but will do a more thorough pass later.
I think it would be good if we can combine or refactor the RANS scripts to avoid code duplication. They are essentially all the same except for a few options. I suggest one of the two,

refactor the geometry generation out into its own file that can be imported or run.
use argparse with argument mode (or similar) to run each case and just update the relevant options based on the mode. This will have a smaller footprint.

eirikurj · 2024-09-04T15:27:54Z

I have updated the files. However, in line with my previous comment on refactoring the scripts I updated the names and inserted rans in the files i.e.,

naca0012_growth_ratios.cgns -> naca0012_rans_growth_ratios.cgns
naca0012_schedule.cgns      -> naca0012_rans_schedule.cgns

To avoid nightly builds failing, I named the archive with the updated files is pyhyp_ref_files_2024-09-04.tar.gz. You can update the script to pull this archive instead for testing. Once ready I will update the main.

DavidAnderegg · 2024-09-26T12:17:12Z

@eirikurj I adjusted the code as explained in my previous comment. It is once again ready for review.

Please note: i also adjusted the description and added new ref-files.

updating MExt to support python 3.12 (mdolab#95)

anilyil

Great work overall. I left some comments. I am really happy with the cleanup you did with fortran, it makes so much more sense to compute those parameters in python.

doc/options.yaml

pyhyp/pyHyp.py

DavidAnderegg · 2024-10-08T08:25:12Z

@anilyil, I adressed your comments. You might want to take a look again.

anilyil

Thanks for the updates. This is good to go for me.

anilyil

@DavidAnderegg we just realized the growth ratios printed with pyHypMulti is broken now. It just prints 0s. Can you look into this?

DavidAnderegg · 2024-10-15T09:16:11Z

@anilyil I know why this happens... Yes, I will look into it.

DavidAnderegg · 2024-10-16T09:13:42Z

@anilyil Grid Ratios for pyHypMulti print now correctly.

I also replaced that code with 'tabulate' because it is so much easier this way.

eirikurj

Sorry for the delay. I did a quick pass and overall the changes are very good. Just a few very minor comments or questions. Let me know if anything is unclear. Also, can you prepare and link an archive with updated reference files for the tests. Want to test this locally and I will also need to upload it to our site.

examples/naca0012/naca0012_rans.py

eirikurj · 2024-10-16T12:15:20Z

pyhyp/pyHyp.py

@@ -405,8 +384,19 @@ def __init__(self, comm=None, options=None, debug=False):
        if options is None:
            raise Error("The options = keyword argument is *NOT* optional. " "It must always be provided")

+        # Deprecated options
+        deprecated_options = self._getDeprecatedOptions()


We use camelCase (there are exceptions) for our codebase. Please change naming (here and elsewhere) to make it consistent.

I will change it.

But are not any of the tools (flake8, black, ...) supposed to catch this?

pyhyp/pyHyp.py

eirikurj · 2024-10-16T12:53:13Z

pyhyp/pyHyp.py

 from baseclasses import BaseSolver
 from baseclasses.utils import Error
 from cgnsutilities.cgnsutilities import readGrid, combineGrids


+class pyHypWarning(object):


While this is fine for now, I just wanted to note that we have been moving towards using the standard library warnings module. You can see baseclasses for examples of usage.

I can move to a standard library. I just copied the idea from ADflow. Let me know what you want?

pyhyp/pyHyp.py

eirikurj · 2024-10-16T14:07:02Z

pyhyp/pyHyp.py

+
+        return fullDeltaS, marchDist, growth_ratios
+
+    def get_growth_ratio_string(self, growth_ratios=None, n_decimals=3):


Add argument descriptions (same for other new functions). Also probably should make it "private" with an underscore, or is there a use case for the user here that I am missing?

I think this depends on your definition of private/public. I understood it like this: only accessed within the class = private; gets accessed outside of the class = public.

This function is called by pyHypMulti, which does not inherit from pyHyp. Thus it is accessed from the outside and should be public.

DavidAnderegg · 2024-11-13T16:43:44Z

@eirikurj I adressed your comments. I only resolved the ones where I knew what to do. The unresolved ones either need an answer or a small discussion.

Regarding the ref files, I edited the PR description and updated the link to the ref-files.

Regarding function description. I left out the ones in the examples. Let me know if I should add it there aswell.

anilyil and others added 20 commits July 10, 2023 21:59

minor change on cart grid

e9654d4

added scheduled smoothing and splay

e0b06c1

added variable growth ratios as well

4274130

Add tests to 'Schedule' parameters

4325440

Update library import checks (mdolab#67)

d5b5cde

Update setup.py + black formatting (mdolab#70)

45baf98

* Update setup.py * changed numpy to <1.24 * ran black * Bump patch version --------- Co-authored-by: sseraj <[email protected]>

update setup.py (mdolab#73)

19492ff

Updated testing docs (mdolab#75)

eda5ea7

Add config file for building docs on RTD (mdolab#74)

1d5fbd3

* add config file for building docs * add newline --------- Co-authored-by: Marco Mangano <[email protected]>

Updated Intel config file to work with HPE MPI (mdolab#76)

f24c42c

* renamed Intel config file * renamed gfortran config file * updated azure script * added MPI if check in Intel config * minor cleanup * updated docs * fixed Intel env var

Add missing test "n0012 schedule"

4df92b4

Print min and max growth ratio if it is user specified

0e4237a

Add test for 'growth_ratios'

3411f3c

Add description to new options

14a2cfd

Merge branch 'main' of https://github.com/mdolab/pyhyp into mdolab-main

ae788a3

Merge branch 'mdolab-main' into vol_blend_schedule

fd282c4

Replace duplicated code with pointer based subroutine

b34e742

pick the smallest grid ratio for pGridRatio

6eb50e7

run 'black' and 'fprettify'

145b44e

Remove trailing white space

13c9818

DavidAnderegg requested a review from a team as a code owner September 4, 2024 13:41

DavidAnderegg requested review from lamkina and eirikurj September 4, 2024 13:41

DavidAnderegg added 3 commits September 5, 2024 08:06

Compute marchdistance when using custom growth ratios

57746df

remove code duplication by merging the rans-tests

357b05f

Run black, fprettify and flake8

ce7e0e0

DavidAnderegg added 2 commits September 26, 2024 11:58

Add test for constant layers at the beginning and end of extrusion

4f29dd7

Update documentation

6a23b58

DavidAnderegg requested a review from eirikurj September 26, 2024 12:16

Merge pull request #2 from mdolab/main

f49268a

updating MExt to support python 3.12 (mdolab#95)

anilyil reviewed Oct 4, 2024

View reviewed changes

doc/options.yaml Show resolved Hide resolved

doc/options.yaml Show resolved Hide resolved

doc/options.yaml Show resolved Hide resolved

pyhyp/pyHyp.py Show resolved Hide resolved

pyhyp/pyHyp.py Outdated Show resolved Hide resolved

Print marchDist and add deprecation warning to 'volSmoothSchedule'

5ff70bd

anilyil self-requested a review October 14, 2024 04:17

anilyil previously approved these changes Oct 14, 2024

View reviewed changes

anilyil self-requested a review October 14, 2024 20:21

anilyil reviewed Oct 14, 2024

View reviewed changes

print the correct gridRatio for pyHypMulti

ae92c75

DavidAnderegg dismissed anilyil’s stale review via ae92c75 October 16, 2024 09:08

extend warning about precedence of explicit 'growthratios'

84e0216

eirikurj requested changes Oct 16, 2024

View reviewed changes

DavidAnderegg added 11 commits November 13, 2024 10:41

fix typos

adc533a

Use member variable instead of passing it into the function

f5f4247

move mach parameters printing into own function

667d140

simplify function

e4ee691

rename default case as such

6c5156c

Refactor to camelCase

e87d1b3

also move tests and missed cases to camelCase

186e813

fix typos

6201c11

Rephrase error message

f9f0824

Add argument descriptions

25ca99f

run black and flake8

6706961

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom GrowthRatios and more Schedule variables #94

Custom GrowthRatios and more Schedule variables #94

DavidAnderegg commented Sep 4, 2024 •

edited

Loading

eirikurj commented Sep 4, 2024

DavidAnderegg commented Sep 4, 2024

eirikurj commented Sep 4, 2024

eirikurj commented Sep 4, 2024 •

edited

Loading

DavidAnderegg commented Sep 26, 2024 •

edited

Loading

anilyil left a comment

DavidAnderegg commented Oct 8, 2024

anilyil left a comment

anilyil left a comment

DavidAnderegg commented Oct 15, 2024

DavidAnderegg commented Oct 16, 2024

eirikurj left a comment

eirikurj Oct 16, 2024

DavidAnderegg Nov 13, 2024

eirikurj Oct 16, 2024

DavidAnderegg Nov 13, 2024

eirikurj Oct 16, 2024

DavidAnderegg Nov 13, 2024

DavidAnderegg commented Nov 13, 2024


		return fullDeltaS, marchDist, growth_ratios

		def get_growth_ratio_string(self, growth_ratios=None, n_decimals=3):

Custom GrowthRatios and more Schedule variables #94

Are you sure you want to change the base?

Custom GrowthRatios and more Schedule variables #94

Conversation

DavidAnderegg commented Sep 4, 2024 • edited Loading

Purpose

Expected time until merged

Type of change

Testing

Checklist

eirikurj commented Sep 4, 2024

DavidAnderegg commented Sep 4, 2024

eirikurj commented Sep 4, 2024

eirikurj commented Sep 4, 2024 • edited Loading

DavidAnderegg commented Sep 26, 2024 • edited Loading

anilyil left a comment

Choose a reason for hiding this comment

DavidAnderegg commented Oct 8, 2024

anilyil left a comment

Choose a reason for hiding this comment

anilyil left a comment

Choose a reason for hiding this comment

DavidAnderegg commented Oct 15, 2024

DavidAnderegg commented Oct 16, 2024

eirikurj left a comment

Choose a reason for hiding this comment

eirikurj Oct 16, 2024

Choose a reason for hiding this comment

DavidAnderegg Nov 13, 2024

Choose a reason for hiding this comment

eirikurj Oct 16, 2024

Choose a reason for hiding this comment

DavidAnderegg Nov 13, 2024

Choose a reason for hiding this comment

eirikurj Oct 16, 2024

Choose a reason for hiding this comment

DavidAnderegg Nov 13, 2024

Choose a reason for hiding this comment

DavidAnderegg commented Nov 13, 2024

DavidAnderegg commented Sep 4, 2024 •

edited

Loading

eirikurj commented Sep 4, 2024 •

edited

Loading

DavidAnderegg commented Sep 26, 2024 •

edited

Loading