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.

Sign up for GitHub

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

Jump to bottom

Replace an always false test with an abort #1655

Merged
merged 2 commits into from
Jan 27, 2021
Merged

Replace an always false test with an abort #1655

merged 2 commits into from
Jan 27, 2021

Conversation

lucafedeli88
Copy link
Member

@lucafedeli88 lucafedeli88 commented Jan 26, 2021 •
edited
Loading

I am not completely sure about replacing an assert(false, msg) with abort(msg). However, adding #else ... #endif also fixes a potential null pointer dereference warning

@lucafedeli88 lucafedeli88 added the cleaning Clean code, improve readability label Jan 26, 2021
@dpgrote
Copy link
Member

dpgrote commented Jan 26, 2021

Where does the potential null pointer deference happen?

@lucafedeli88
Copy link
Member Author

lucafedeli88 commented Jan 26, 2021 •
edited
Loading

Where does the potential null pointer deference happen?

here:

./Source/Utils/WarpXUtil.cpp: In function ‘void CheckGriddingForRZSpectral()’:
./Source/Utils/WarpXUtil.cpp:299:24: warning: potential null pointer dereference [-Wnull-dereference]
  299 |     max_grid_size_x[0] = n_cell[0];

I compiled with make QED=TRUE DIM=2 COMP=llvm

@dpgrote
Copy link
Member

dpgrote commented Jan 26, 2021

Does it also complain about line 298?

The change looks good - I'll approve it.

dpgrote
dpgrote approved these changes Jan 26, 2021
View reviewed changes
Copy link
Member

@dpgrote dpgrote left a comment

Choose a reason for hiding this comment

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

Thanks for making the change!

@lucafedeli88 lucafedeli88 merged commit 71005b3 into ECP-WarpX:development Jan 27, 2021
mrowan137 pushed a commit to mrowan137/WarpX that referenced this pull request Feb 2, 2021
@EZoni @mrowan137
Fix bug in momentum-conserving interpolation function with MR (ECP-Wa…
2850fed 
…rpX#1624)

* Fix bug in momentum-conserving interpolation function

* Reset checksum benchmark for CI test momentum-conserving-gather

Unused Params: Check after Step 0 (ECP-WarpX#1596)

An early check for unused parameters after the first step.

Add Superparticle version of getParticlePosition. (ECP-WarpX#1640)

* Add Superparticle version of GetParticlePosition.
* move unpack_particle to a free function and rename
* no longer need SuperPType
* Update Source/Particles/Pusher/GetAndSetPosition.H
* remove templating from get_particle_position
* remove template
* Add missing include

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>

Distribution mapping and cost plotting (ECP-WarpX#1444)

* Distribution mapping and cost plotting

Cleanup

Cleanup

Cleanup

EOL

Unused import

* Cost initializes to 0.0

* plot slices of 3D

* WIP

* WIP docs

* docs

* docs

* docs

* docs

* docs

* docs

* docs

* docs

* EOL

SpeciesProperties: C, N, O, Cu (ECP-WarpX#1638)

* SpeciesProperties: C, N, O, Cu

Add more ionic species as pre-defined particle species.

* Cu: Add to Ionization Energies

Fix 2D: ParticleHistogram (ECP-WarpX#1635)

Fix out-of-bounds access in particle histogram in non-3D.

Fix FilterFunctor in non-3D (ECP-WarpX#1633)

Fix an out-of-bounds access to positions in 2D & RZ for
filter functors.

openPMD: ionizationLevel (ECP-WarpX#1622)

* openPMD: ionizationLevel

Write out the ionizationLevel with openPMD.

* openPMD Record Repetitions: Use Lambdas

openPMD: Particle Filter (Container) (ECP-WarpX#1632)

* openPMD: Particle Filter (Container)

A fresh implementation of particle filters for openPMD diagnostics
using the generalized `PhysicalParticleContainer` approach that we
also use in plotfiles 🎉

* Ion Example: Filter & Coarsen

Test diagnostics filter & coarsen functions in CI.
Test reduced diagnostics (histograms).

Define: _OPENMP -> AMREX_USE_OMP (ECP-WarpX#1520)

* Define: _OPENMP -> AMREX_USE_OMP

Replace the define check of `_OPENMP` with the explicit
backend control of `AMREX_USE_OMP` for parallel constructs.

Doing so avoids that we accidentially turn on OpenMP, e.g. if a dependency
pulls it in for linear algebra, I/O, etc. This can led to confusion if the
user explicitly requested a serial build. Also, we might want to use OpenMP
functionality here and there for auxiliary functions w/o having to use the
AMReX OpenMP backend, i.e. because we compile for GPUs.

* Add missing amrex::Gpu::notInLaunchRegion

Start: PerformanceHints After Init (ECP-WarpX#1630)

Start a helper routine that gives performance hints after
initialization of the simulation.

Fix number of guard cells in PML with 2D PSATD (ECP-WarpX#1648)

Remove unused lines about particle filters (ECP-WarpX#1646)

Add parameter for default galilean velocity (ECP-WarpX#1097)

* Add parameter for default galilean velocity

* Apply suggestions from code review

Co-authored-by: Edoardo Zoni <59625522+EZoni@users.noreply.github.com>

* Fix bug in constructor of PhysicalParticleContainer

* Use new input parameter in CI test

Co-authored-by: Edoardo Zoni <59625522+EZoni@users.noreply.github.com>
Co-authored-by: Edoardo Zoni <ezoni@lbl.gov>

Cleaned up field diagnostic in picmi interface (ECP-WarpX#1649)

Use pinned memory for tmp particles in diags. (ECP-WarpX#1644)

* Use pinned memory for tmp particles in diags.

* openPMD: pinned memory (tmp particles)

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>

Replace an always false test with an abort (ECP-WarpX#1655)

* replace always false test with abort

* fix bug

[mini-PR] Broken link to scripts (ECP-WarpX#1651)

CI: Ubuntu-latest -> 20.04 (ECP-WarpX#1654)

Start Embedded Boundary development (ECP-WarpX#1641)

* Start Embedded Boundary development

Modify the build system for embedded boundary (EB) support.  Currently EB is
a compile time option that must be explicitly enabled.  For GNU Make, one
can enable it with `USE_EB=TRUE`, whereas for CMake, `-DAMReX_EB=ON`.  Later
we could decide to enable EB by default with all regular geometry.

Add a simple geometry initialization function, WarpX::InitEB.  By default,
the geometry is all regular.  A few basic types such as box, cylinder,
plane, sphere, etc. are supported via ParmParse runtime parameters.  See
`amrex/Src/EB/AMReX_EB2.cpp` for more details.  Later, we could build more
complex geometry using constructive solid geometry (CSG).  (
https://en.wikipedia.org/wiki/Constructive_solid_geometry ) See
`amrex/Tutorials/EB/GeometryGeneration` for an example of CSG.  There is
also a STL ( https://en.wikipedia.org/wiki/STL_(file_format) ) approach
under development in AMReX.

Add a new member, m_factory, to WarpX class.  This object can be used to
obtain geometry information such as whether a cell is cut, edge centroids,
etc.  Currently we are not using these factories to build MultiFabs for
field data to embed the geometry information into the data containers.  We
could do that later if it is needed or it makes things more convenient.
Nevertheless, this should be sufficient to start the EB development of the
field solver.

It's not clear to me yet how many ghost cells are needed for the geometry
information.  It's currently one, and can be adjusted.

In the future, when particle and embedded boundary interaction is
considered, we can use `amrex::FillSignedDistance` function to obtain signed
distance function on the nodes.  With that information, one should be able
to determine where and when a particle collides with the embedded boundary.

* resize factory vector

* CMake: Require AMReX_EB

for embedded boundaries

* Add AMReX_Config.H et al.

Explicit includes are most robust to make sure defines are set with
future refactorings.

* rename Factory fieldFactory

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>

Add filter for particle histogram (ECP-WarpX#1643)

* Add filter for particle histogram

* Update benchmarks

* Update benchmark again

* Use serialize_ics in test + proper usage of RandomEngine

* Laser-Ion Acc. (2D3V): Test Hist Filter

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>

Take time step into account to compute guard cells for J and rho (ECP-WarpX#1607)

* Use IntVect for ng_J and ng_rho

* Compute guard cells for J and rho based on dt

* Reset some CI benchmarks

* Fix rebase commit

* Add back +1 cell for rho: fix remaining out-of-bound accesses

* Simplify ASSERTS using new interface of amrex::numParticlesOutOfRange

Modern setup.py: CMake-Driven, Multi-Dimensional (ECP-WarpX#1647)

This adds a new, project-centric setup.py file.
With this file, all dimensions (2D, 3D, RZ) of WarpX can be built
and packaged at once, using the CMake build logic.

Build & install:
```bash
pip wheel -v .
pip install *whl
```

Updates for electrostatic solver (ECP-WarpX#1604)

* Fix electrostatic solver with momentum conservation

* Fix electrostatic, adding call to FillBoundaryAux

* For electrostatic, removed unneeded extra calls to UpdateAux and FillBoundary

* For electrostatic, calculate fields at the end of the time step

* Updated ElectrostaticSphere analysis script to use fields from end of time step

Reset broken benchmark for CI test initial_distribution (ECP-WarpX#1661)

make sure we redefine the tmp particle tiles when we load balance. (ECP-WarpX#1658)

[mini-PR] Add particle tiling documentation (ECP-WarpX#1665)

* tiling wip

* do_tiling documentation

* Update Docs/source/running_cpp/parameters.rst

Co-authored-by: Luca Fedeli <luca.fedeli@for.unipi.it>

Co-authored-by: Luca Fedeli <luca.fedeli@for.unipi.it>

fix inconsistent formatting (ECP-WarpX#1666)

parameter.rst fixes (ECP-WarpX#1663)

requirements.txt: fix overspecification (ECP-WarpX#1668)

I accidentally added the patch-level for version-compatible matching

This removes the patch-level for `~=` matching for the packages that
have a >=1 major version already.

scipy 1.5+ (ECP-WarpX#1670)

Sufficient since it was last release in december and works well.
The 3.5 release series still builds wheels for Python 3.6 for Ubuntu
oldstable (18.04).

Remove redundant "do_qed" option in inputfile (ECP-WarpX#1667)

* removed redundant do_qed option in inputfile

* fixed bug

remove semicolons (ECP-WarpX#1662)

fixTypo for pml has particles parameter for docs (ECP-WarpX#1671)

load balance efficiency reduced diagnostic

eol

minor

eol
mrowan137 pushed a commit to mrowan137/WarpX that referenced this pull request Feb 3, 2021
@EZoni @mrowan137
Fix bug in momentum-conserving interpolation function with MR (ECP-Wa…
365831d 
…rpX#1624)

* Fix bug in momentum-conserving interpolation function

* Reset checksum benchmark for CI test momentum-conserving-gather

Unused Params: Check after Step 0 (ECP-WarpX#1596)

An early check for unused parameters after the first step.

Add Superparticle version of getParticlePosition. (ECP-WarpX#1640)

* Add Superparticle version of GetParticlePosition.
* move unpack_particle to a free function and rename
* no longer need SuperPType
* Update Source/Particles/Pusher/GetAndSetPosition.H
* remove templating from get_particle_position
* remove template
* Add missing include

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>

Distribution mapping and cost plotting (ECP-WarpX#1444)

* Distribution mapping and cost plotting

Cleanup

Cleanup

Cleanup

EOL

Unused import

* Cost initializes to 0.0

* plot slices of 3D

* WIP

* WIP docs

* docs

* docs

* docs

* docs

* docs

* docs

* docs

* docs

* EOL

SpeciesProperties: C, N, O, Cu (ECP-WarpX#1638)

* SpeciesProperties: C, N, O, Cu

Add more ionic species as pre-defined particle species.

* Cu: Add to Ionization Energies

Fix 2D: ParticleHistogram (ECP-WarpX#1635)

Fix out-of-bounds access in particle histogram in non-3D.

Fix FilterFunctor in non-3D (ECP-WarpX#1633)

Fix an out-of-bounds access to positions in 2D & RZ for
filter functors.

openPMD: ionizationLevel (ECP-WarpX#1622)

* openPMD: ionizationLevel

Write out the ionizationLevel with openPMD.

* openPMD Record Repetitions: Use Lambdas

openPMD: Particle Filter (Container) (ECP-WarpX#1632)

* openPMD: Particle Filter (Container)

A fresh implementation of particle filters for openPMD diagnostics
using the generalized `PhysicalParticleContainer` approach that we
also use in plotfiles 🎉

* Ion Example: Filter & Coarsen

Test diagnostics filter & coarsen functions in CI.
Test reduced diagnostics (histograms).

Define: _OPENMP -> AMREX_USE_OMP (ECP-WarpX#1520)

* Define: _OPENMP -> AMREX_USE_OMP

Replace the define check of `_OPENMP` with the explicit
backend control of `AMREX_USE_OMP` for parallel constructs.

Doing so avoids that we accidentially turn on OpenMP, e.g. if a dependency
pulls it in for linear algebra, I/O, etc. This can led to confusion if the
user explicitly requested a serial build. Also, we might want to use OpenMP
functionality here and there for auxiliary functions w/o having to use the
AMReX OpenMP backend, i.e. because we compile for GPUs.

* Add missing amrex::Gpu::notInLaunchRegion

Start: PerformanceHints After Init (ECP-WarpX#1630)

Start a helper routine that gives performance hints after
initialization of the simulation.

Fix number of guard cells in PML with 2D PSATD (ECP-WarpX#1648)

Remove unused lines about particle filters (ECP-WarpX#1646)

Add parameter for default galilean velocity (ECP-WarpX#1097)

* Add parameter for default galilean velocity

* Apply suggestions from code review

Co-authored-by: Edoardo Zoni <59625522+EZoni@users.noreply.github.com>

* Fix bug in constructor of PhysicalParticleContainer

* Use new input parameter in CI test

Co-authored-by: Edoardo Zoni <59625522+EZoni@users.noreply.github.com>
Co-authored-by: Edoardo Zoni <ezoni@lbl.gov>

Cleaned up field diagnostic in picmi interface (ECP-WarpX#1649)

Use pinned memory for tmp particles in diags. (ECP-WarpX#1644)

* Use pinned memory for tmp particles in diags.

* openPMD: pinned memory (tmp particles)

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>

Replace an always false test with an abort (ECP-WarpX#1655)

* replace always false test with abort

* fix bug

[mini-PR] Broken link to scripts (ECP-WarpX#1651)

CI: Ubuntu-latest -> 20.04 (ECP-WarpX#1654)

Start Embedded Boundary development (ECP-WarpX#1641)

* Start Embedded Boundary development

Modify the build system for embedded boundary (EB) support.  Currently EB is
a compile time option that must be explicitly enabled.  For GNU Make, one
can enable it with `USE_EB=TRUE`, whereas for CMake, `-DAMReX_EB=ON`.  Later
we could decide to enable EB by default with all regular geometry.

Add a simple geometry initialization function, WarpX::InitEB.  By default,
the geometry is all regular.  A few basic types such as box, cylinder,
plane, sphere, etc. are supported via ParmParse runtime parameters.  See
`amrex/Src/EB/AMReX_EB2.cpp` for more details.  Later, we could build more
complex geometry using constructive solid geometry (CSG).  (
https://en.wikipedia.org/wiki/Constructive_solid_geometry ) See
`amrex/Tutorials/EB/GeometryGeneration` for an example of CSG.  There is
also a STL ( https://en.wikipedia.org/wiki/STL_(file_format) ) approach
under development in AMReX.

Add a new member, m_factory, to WarpX class.  This object can be used to
obtain geometry information such as whether a cell is cut, edge centroids,
etc.  Currently we are not using these factories to build MultiFabs for
field data to embed the geometry information into the data containers.  We
could do that later if it is needed or it makes things more convenient.
Nevertheless, this should be sufficient to start the EB development of the
field solver.

It's not clear to me yet how many ghost cells are needed for the geometry
information.  It's currently one, and can be adjusted.

In the future, when particle and embedded boundary interaction is
considered, we can use `amrex::FillSignedDistance` function to obtain signed
distance function on the nodes.  With that information, one should be able
to determine where and when a particle collides with the embedded boundary.

* resize factory vector

* CMake: Require AMReX_EB

for embedded boundaries

* Add AMReX_Config.H et al.

Explicit includes are most robust to make sure defines are set with
future refactorings.

* rename Factory fieldFactory

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>

Add filter for particle histogram (ECP-WarpX#1643)

* Add filter for particle histogram

* Update benchmarks

* Update benchmark again

* Use serialize_ics in test + proper usage of RandomEngine

* Laser-Ion Acc. (2D3V): Test Hist Filter

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>

Take time step into account to compute guard cells for J and rho (ECP-WarpX#1607)

* Use IntVect for ng_J and ng_rho

* Compute guard cells for J and rho based on dt

* Reset some CI benchmarks

* Fix rebase commit

* Add back +1 cell for rho: fix remaining out-of-bound accesses

* Simplify ASSERTS using new interface of amrex::numParticlesOutOfRange

Modern setup.py: CMake-Driven, Multi-Dimensional (ECP-WarpX#1647)

This adds a new, project-centric setup.py file.
With this file, all dimensions (2D, 3D, RZ) of WarpX can be built
and packaged at once, using the CMake build logic.

Build & install:
```bash
pip wheel -v .
pip install *whl
```

Updates for electrostatic solver (ECP-WarpX#1604)

* Fix electrostatic solver with momentum conservation

* Fix electrostatic, adding call to FillBoundaryAux

* For electrostatic, removed unneeded extra calls to UpdateAux and FillBoundary

* For electrostatic, calculate fields at the end of the time step

* Updated ElectrostaticSphere analysis script to use fields from end of time step

Reset broken benchmark for CI test initial_distribution (ECP-WarpX#1661)

make sure we redefine the tmp particle tiles when we load balance. (ECP-WarpX#1658)

[mini-PR] Add particle tiling documentation (ECP-WarpX#1665)

* tiling wip

* do_tiling documentation

* Update Docs/source/running_cpp/parameters.rst

Co-authored-by: Luca Fedeli <luca.fedeli@for.unipi.it>

Co-authored-by: Luca Fedeli <luca.fedeli@for.unipi.it>

fix inconsistent formatting (ECP-WarpX#1666)

parameter.rst fixes (ECP-WarpX#1663)

requirements.txt: fix overspecification (ECP-WarpX#1668)

I accidentally added the patch-level for version-compatible matching

This removes the patch-level for `~=` matching for the packages that
have a >=1 major version already.

scipy 1.5+ (ECP-WarpX#1670)

Sufficient since it was last release in december and works well.
The 3.5 release series still builds wheels for Python 3.6 for Ubuntu
oldstable (18.04).

Remove redundant "do_qed" option in inputfile (ECP-WarpX#1667)

* removed redundant do_qed option in inputfile

* fixed bug

remove semicolons (ECP-WarpX#1662)

fixTypo for pml has particles parameter for docs (ECP-WarpX#1671)

Move getCosts

Always synchronize nodal points of PML MultiFabs (ECP-WarpX#1669)

* Always synchronize nodal points of PML MultiFabs

* Reset benchmark for CI test pml_x_psatd

* Use new separate functions NodalSyncPML

minor
@lucafedeli88 lucafedeli88 deleted the replace_always_false_test branch February 4, 2021 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dpgrote dpgrote dpgrote approved these changes

Assignees
No one assigned
Labels
cleaning Clean code, improve readability
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lucafedeli88 @dpgrote