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

added lots of if allocated statements #178

Merged
merged 4 commits into from
Feb 2, 2022
Merged

Conversation

joanibal
Copy link
Collaborator

Purpose

If ADflow has an issue in its init it will often try to deallocate variables that haven't been allocated yet. This will usually obscure the original error. This was particularly annoying for me since I was working on pyAeroProblem, which is used before the preprocessing has been complete.

To solve this I added lots of if (allocated(blah)) statements to prevent it from deallocating variables that don't exist yet.

Before

python test_basics.py
#
# ADflow, multiblock structured flow solver
#
# This code solves the 3D RANS, laminar NS or Euler equations
# on multiblock structured hexahedral grids.
# This is a parallel executable running on 1 processors.
# It has been compiled with the following options:
# - Optimized mode.
# - Size of standard integers: 4 bytes.
# - Size of standard floating point types: 8 bytes.
# - With cgns support
# - With support for signals.
#
-> Alpha... 0.000000 
E[0]PETSC ERROR: ------------------------------------------------------------------------
[0]PETSC ERROR: Caught signal number 11 SEGV: Segmentation Violation, probably memory access out of range
[0]PETSC ERROR: Try option -start_in_debugger or -on_error_attach_debugger
[0]PETSC ERROR: or see http://www.mcs.anl.gov/petsc/documentation/faq.html#valgrind
[0]PETSC ERROR: or try http://valgrind.org on GNU/linux and Apple Mac OS X to find memory corruption errors
[0]PETSC ERROR: likely location of problem given in stack below
[0]PETSC ERROR: ---------------------  Stack Frames ------------------------------------
[0]PETSC ERROR: Note: The EXACT line numbers in the stack are not available,
[0]PETSC ERROR:       INSTEAD the line number of the start of the function
[0]PETSC ERROR:       is given.
--------------------------------------------------------------------------
MPI_ABORT was invoked on rank 0 in communicator MPI_COMM_WORLD
with errorcode 59.

NOTE: invoking MPI_ABORT causes Open MPI to kill all MPI processes.
You may or may not see output from other processes, depending on
exactly when Open MPI kills them.
--------------------------------------------------------------------------

or

> python test_basics.py
#
# ADflow, multiblock structured flow solver
#
# This code solves the 3D RANS, laminar NS or Euler equations
# on multiblock structured hexahedral grids.
# This is a parallel executable running on 1 processors.
# It has been compiled with the following options:
# - Optimized mode.
# - Size of standard integers: 4 bytes.
# - Size of standard floating point types: 8 bytes.
# - With cgns support
# - With support for signals.
#
-> Alpha... 0.000000 
E ---------------------------------------------------------------------------
PETSc or MPI Error. Error Code 64. Detected on Proc  0
Error at line:  4794 in file: ../utils/utils.F90
 ---------------------------------------------------------------------------
--------------------------------------------------------------------------
MPI_ABORT was invoked on rank 0 in communicator MPI_COMM_WORLD
with errorcode 64.

NOTE: invoking MPI_ABORT causes Open MPI to kill all MPI processes.
You may or may not see output from other processes, depending on
exactly when Open MPI kills them.
--------------------------------------------------------------------------

After

> python test_basics.py
#
# ADflow, multiblock structured flow solver
#
# This code solves the 3D RANS, laminar NS or Euler equations
# on multiblock structured hexahedral grids.
# This is a parallel executable running on 1 processors.
# It has been compiled with the following options:
# - Optimized mode.
# - Size of standard integers: 4 bytes.
# - Size of standard floating point types: 8 bytes.
# - With cgns support
# - With support for signals.
#
+----------------------------------------+
|          All ADFLOW Options:           |
+----------------------------------------+
{....}
-> Alpha... 0.000000 
E
======================================================================
ERROR: test_import (__main__.BasicTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_basics.py", line 15, in test_import
    ADFLOW(options=options, debug=False)
  File "/home/josh/repos/adflow/adflow/pyADflow.py", line 224, in __init__
    self._setAeroProblemData(dummyAP, firstCall=True)
  File "/home/josh/repos/adflow/adflow/pyADflow.py", line 3006, in _setAeroProblemData
    nameArray, dataArray, groupArray, groupNames, empty = self._getBCDataFromAeroProblem(AP)
  File "/home/josh/repos/adflow/adflow/pyADflow.py", line 3023, in _getBCDataFromAeroProblem
    for tmp in AP.bcVarData:
AttributeError: 'AeroProblem' object has no attribute 'bcVarData'

----------------------------------------------------------------------
Ran 1 test in 0.013s

FAILED (errors=1)

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)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Documentation update
  • Maintenance update
  • Other (please describe)

Testing

Checklist

  • I have run flake8 and black to make sure the code adheres to PEP-8 and is consistently formatted
  • 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

@joanibal joanibal requested a review from a team as a code owner January 26, 2022 17:57
@joanibal joanibal linked an issue Jan 26, 2022 that may be closed by this pull request
@joanibal joanibal added enhancement New feature or request maintenance This is for maintaining the repo labels Jan 26, 2022
@joanibal
Copy link
Collaborator Author

Why do we use flake8 and pylint? @marcomangano @nwu63
pylint seems upset about files that were not modified by this PR.

@ewu63
Copy link
Collaborator

ewu63 commented Jan 26, 2022

Why do we use flake8 and pylint? @marcomangano @nwu63 pylint seems upset about files that were not modified by this PR.

See mdolab/.github#22 for the discussion on pylint. It catches many problems that flake8 doesn't catch. Since nobody went through and fixed these problems, it's going to fail the check for pretty much all the repos, which is why it's not a required status check.

@codecov
Copy link

codecov bot commented Jan 26, 2022

Codecov Report

Merging #178 (960389f) into master (d17ca9f) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 960389f differs from pull request most recent head 37f573a. Consider uploading reports for the commit 37f573a to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #178   +/-   ##
=======================================
  Coverage   43.10%   43.10%           
=======================================
  Files          15       15           
  Lines        3517     3517           
=======================================
  Hits         1516     1516           
  Misses       2001     2001           
Impacted Files Coverage Δ
adflow/pyADflow.py 67.46% <100.00%> (ø)

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 d17ca9f...37f573a. Read the comment docs.

Copy link
Collaborator

@sseraj sseraj left a comment

Choose a reason for hiding this comment

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

Excellent! I tested this locally and it works.

@ewu63 ewu63 requested a review from awccopp January 27, 2022 16:38
Copy link
Contributor

@awccopp awccopp left a comment

Choose a reason for hiding this comment

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

Everything here looks good to me.

@ewu63 ewu63 merged commit 4ed1f90 into mdolab:master Feb 2, 2022
@joanibal joanibal deleted the deallocate branch March 28, 2022 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request maintenance This is for maintaining the repo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Better error handling during setup
4 participants