-
Notifications
You must be signed in to change notification settings - Fork 42
Custom GrowthRatios and more Schedule variables #94
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
Conversation
* 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
@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. |
@eirikurj The tests fail because new the ref files have not been added. I attached a .zip in the description of this PR. |
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 have updated the files. However, in line with my previous comment on refactoring the scripts I updated the names and inserted
To avoid nightly builds failing, I named the archive with the updated files is |
@DavidAnderegg, does the new files break old tests or did you only add new files for the new tests? if we are not breaking any of the existing tests, I can upload the ref files now. If they break existing tests, we can attempt to do it within the same day we plan on merging the PR |
@anilyil The new ref files do break the old tests. This is because I extended the 'corner' case. So I suggest you wait with updating until the day this PR gets merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the changes look good, see my comments for just a few clarifications.
The new reference archive attached was missing the naca0012_rans_schedule.cgns
file. In order to get this merged as soon as possible, I just created this file and updated and uploaded the reference archive pyhyp_ref_files_20241210.tar.gz
to our servers. I have pulled this archive locally and the tests pass.
However, this process is annoying and to avoid this issue of failing tests on GH due to reference files not up-to-date, we can just change get-ref-files.sh
to point to the dated reference file. This is a better long term strategy as this allows current and new reference files and we dont need to change anything once merged.
@eirikurj I am sorry for the missing ref-file, I did not realize it... I addressed the comments that were clear. I also changed Please let me know what you think regarding the not resolved points. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #94 +/- ##
==========================================
+ Coverage 79.24% 83.64% +4.39%
==========================================
Files 4 4
Lines 424 538 +114
==========================================
+ Hits 336 450 +114
Misses 88 88
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Regarding the failed tests. This is very strange to me. Everything passes except for Edit: |
significant figures rounding could for large numbers.
Looks good, thanks for all your efforts on this! |
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 withvolSmoothIter
.Lastly, a new variable
growthRatios
is introduced to explicitly set the growth ratio. If it is set, themarchDist
option is ignored. This variable works like the other changes introduce here.Expected time until merged
1 week
Type of change
volSmoothSchedule
is now obsolete and was removedTesting
I adjusted the
naca0012_rans
andcorner
test to include an 'interpolation' and a 'per-layer' case. I also added a test fornConstantStart
andnConstantEnd
options. And finally, i added a test forsimpleOCart
withxBounds
and no nearfield mesh.Here are the new ref-files for the new tests: schedule_ref_files.zipHere are the new ref-files for the new tests: refFiles.zip
Checklist
flake8
andblack
to make sure the Python code adheres to PEP-8 and is consistently formattedfprettify
or C/C++ code withclang-format
as applicable