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

New update stack2 #1246

Merged
merged 55 commits into from
Feb 27, 2021
Merged

New update stack2 #1246

merged 55 commits into from
Feb 27, 2021

Conversation

cekees
Copy link
Member

@cekees cekees commented Feb 19, 2021

Mandatory Checklist

Please ensure that the following criteria are met:

  • Title of pull request describes the changes/features
  • Request at least 2 reviewers
  • If new files are being added, the files are no larger than 100kB. Post the file sizes.
  • Code coverage did not decrease. If this is a bug fix, a test should cover that bug fix. If a new feature is added, a test should be made to cover that feature.
  • New features have appropriate documentation strings (readable by sphinx)
  • Contributor has read and agreed with CONTRIBUTING.md and has added themselves to CONTRIBUTORS.md

As a general rule of thumb, try to follow PEP8 guidelines.

Description

This is a major infrastructure update, but it doesn't change any frequently used functionality in Proteus proper. The coverage is going to show up as reduced, but too much non-proteus package code was getting included in the coverage computation. Moving forward we should be able to focus on increasing the coverage on the package itself and further removing unused code and dependencies as well as tests that are being skipped on any platforms.

@codecov
Copy link

codecov bot commented Feb 20, 2021

Codecov Report

Merging #1246 (ff261e1) into main (64d5e85) will decrease coverage by 5.26%.
The diff coverage is 42.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1246      +/-   ##
==========================================
- Coverage   52.82%   47.55%   -5.27%     
==========================================
  Files         531       90     -441     
  Lines      109802    71766   -38036     
==========================================
- Hits        58004    34130   -23874     
+ Misses      51798    37636   -14162     
Impacted Files Coverage Δ
proteus/Archiver.py 36.18% <ø> (+4.54%) ⬆️
proteus/__init__.py 26.08% <ø> (ø)
proteus/MeshTools.py 55.83% <36.11%> (+0.55%) ⬆️
proteus/FemTools.py 38.10% <60.00%> (+0.07%) ⬆️
proteus/Quadrature.py 97.71% <100.00%> (+<0.01%) ⬆️
proteus/TwoPhaseFlow/utils/Parameters.py 88.82% <0.00%> (-0.24%) ⬇️
proteus/tests/CLSVOF/with_RANS3PF/clsvof_p.py
proteus/tests/CLSVOF/with_RANS3PF/ls_consrv_n.py
proteus/tests/levelset/vortex2D/vortex2D.py
...eus/tests/levelset/vortex/ls_consrv_vortex_3d_n.py
... and 432 more

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 64d5e85...ff261e1. Read the comment docs.

@cekees
Copy link
Member Author

cekees commented Feb 23, 2021

@adimako one thing I found that would be a small job to increased portability and remove dependencies would be to remove the curses.h code in the Fenton library. It's not a big deal if the interactive, text-based selection of Fenton theory parameters is useful (though in the long run it might be easier to reimplement that interface code using python and leave the C++ just for the computations.

@cekees
Copy link
Member Author

cekees commented Feb 23, 2021

@ejtovar LFS is working fine for me on both mac os x and linux. I did have some headaches getting all the files properly out of git and into LFS with the proper gitattributes set. I decided that we're never going to be free of binary and large data files for testing, so I'd rather just consistently use LFS than keep those file types in the repository. If you need to add csv, npy, bin, etc. Just do git lfs track filename before your do git lfs add. Or if you accidently add something, do git rm --cached filename; git commit; before adding it back properly.

@@ -406,7 +406,7 @@ namespace proteus



inline double* __cpp_uWindow(double* U, double x[nDim], double x0[nDim], double t, double* t0, double* kDir, double* kAbs, double* omega, double* phi, double* amplitude, double mwl, double depth, int N,int Nw, double* waveDir, double* vDir, double* tanhF, double gAbs , bool fast)
inline void __cpp_uWindow(double* U, double x[nDim], double x0[nDim], double t, double* t0, double* kDir, double* kAbs, double* omega, double* phi, double* amplitude, double mwl, double depth, int N,int Nw, double* waveDir, double* vDir, double* tanhF, double gAbs , bool fast)
Copy link
Member Author

Choose a reason for hiding this comment

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

@adimako I found this pernicious little bug in the optimized WaveTools.h code. My understanding is that you designed the _u* functions to have no return value but this one declared it as double*. I checked over the module and didn't find any other cases, but you might want to take a quick look. It generates a seg fault when optimization is on for the newer compilers. @ejtovar I suspect this may be what causes random waves test to fail on the conda stacks. Note, I also found an issue like this in the optimized quadrilateral mesh generation. With these bug fix commits I also reset the hashdist test compile options to -O2 to verify that all the tests run with full optimization on the hashdist stack.

@cekees cekees merged commit 11d8749 into erdc:main Feb 27, 2021
@cekees cekees deleted the new_update_stack2 branch February 27, 2021 02:31
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

Successfully merging this pull request may close these issues.

2 participants