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

d/fcompare fix #1

Closed
wants to merge 489 commits into from
Closed

d/fcompare fix #1

wants to merge 489 commits into from

Conversation

sayerhs
Copy link
Owner

@sayerhs sayerhs commented Nov 14, 2020

OscarAntepara and others added 30 commits August 10, 2020 11:17
…1235)

## Summary
Computation at inflow boundaries for the MLEBTensor and inflow/outflow for MLTensor has been changed.

## Additional background
At outflow the tensor solve uses extrapolated values from the interior cell to the outflow face. At inflow, the tensor solve uses boundary data, evaluated at the face, that has been stored in the ghost cell next to the inflow face.

## Checklist

The proposed changes:
- [x] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [x] changes answers in the test suite to more than roundoff level
- [x] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
Remove Perilla because it's incompatible with our GPU strategy and it's no
longer being maintained.
PR AMReX-Codes#1224 introduced a bug that affects FillPatch on nodal grids for EB.
When there is only one ghost cell, the fine patch grids might be degenerate
in the sense that some boxes are only 1 node wide.  In that case, we cannot
build cell-centered EBCellFlags unless there are ghost cells.
…es#1254)

## Summary
Correcting tensor cross terms computation for periodic bcs

## Additional background
Fixing a bug introduced in PR AMReX-Codes#1235. Now it computes in the correct way the cross terms in the tensor solve for problems with periodic bcs. 

## Checklist

The proposed changes:
- [x] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [x] changes answers in the test suite to more than roundoff level
- [x] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
…=3. (AMReX-Codes#1256)

## Summary

Warnings from compiling Amrvis using GCC 7.5 with DEBUG=TRUE, DIM=2 and DIM=3. (Build on dogora). Primarily ignore_unused, commenting out function parameter declarations and a few manual type casts. Some unique warning adjustments as well (e.g. a shadowed BL_PROFILER).

To be paired with a matching PR in the Amrvis repo.

## Additional background

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
## Summary
Fix a semicoarsening bug for nodal solver.
## Additional background
In avgdown coefficient, there is a place use coarsen ratio instead of the mg_coarsen_ratio_vec. Also, build the semi_avgdown for nodal solver. 
## Checklist

The proposed changes:
- [x] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
…X-Codes#1236)

This should lead to more useful measurements for Castro.  

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [x] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
now located at `/opt/intel/oneapi/setvars.sh`
Due to issues with pointing point precision, there are points inside the `ProbDomain()` that do not map to cells inside the `Domain()` box when binned using something like the `getParticleCell` function. This PR adds an additional domain, the "RoundoffDomain()", to the Geometry object. Points inside the roundoff domain are always sure to map to cells inside `Domain()`. The bounds are calculated when the Geometry object is constructed using bisection. 

This helps solve a number of issues that can happen when you have particles very close to the domain boundaries.

While I have tested this with Nyx and WarpX, there is a chance that there could be a downstream effect on other codes. If the bisection fails when setting the roundoff domain on some problem, then we may need to adjust some tolerances.
## Summary

`-MM` is not supported for nvcc <= 10.0.

## Checklist

The proposed changes:
- [x] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
## Summary

## Additional background

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
## Summary

ParmParse parameter eb2.extend_domain_face is now true by default.  The
reason for the change is that for most applications it's hard to imagine one
would want it to be false.  It will also solve the EB tensor solver issue
when there is a cut cell just outside the domain abutting a covered valid
cell.

Recent changes to the tensor solvers are incorrect and are reverted here.
They are incorrect because the ghost cells in the modified functions
actually have values at the cell centered.  Although the users put domain
face values in the ghost cells, that is not what the solver does.  The
solver stores the boundary values in boundary registers.  Before the stencil
is being applied, bc function is called to fill the ghost cells with
properly interpolated or extrapolated values so that the stencil operations
are just like working on normal interior cells.

Revert "Correcting tensor cross terms computation for periodic bcs (AMReX-Codes#1254)"
This reverts commit 1197adc.

Revert "Changing computation at inflow/outflow for tensor solve (AMReX-Codes#1235)"
This reverts commit b391885.

Revert "Changing computation at outflow for the EBTensor (AMReX-Codes#1187)"
This reverts commit 9cd5388.

## Checklist

The proposed changes:
- [x] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [x] changes answers in the test suite to more than roundoff level
- [x] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
## Summary

Update make system for hip-clang.  Remove some hip workarounds.

## Additional background

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
## Summary

This replaces the existing tutorial with the more general, all C++ one developed for ATPESC 2020.  This version works for CPU and GPU (compile-time option), in 2D and 3D (compile-time option), with and without subcycling (run-time option).
A .md file is also included in the directory which walks a new user through how to run this code and what to experiment with to understand AMR.

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [x] are described in the proposed changes to the AMReX documentation, if appropriate


Co-authored-by: Weiqun Zhang <weiqunzhang@lbl.gov>
…eX-Codes#1269)

## Summary
changed Tools/GNUMake/comps/cray.mak to fix bugs in the matching of $(COMP_VERSION) in order to correctly treat CCE version > 9

## Additional background
Starting with version 9, the Cray CCE C and C++ compilers changed to being clang/LLVM based
which makes it necessary to change some flags compared to CCE versions <=8.
(This change does not apply to the Cray Fortran compiler versions.)
However, the original version check of
    COMP_VERSION = $(shell echo $(CRAY_CC_VERSION) | cut -f 1 -d .)
in combination with the tests of  "ifeq ($(COMP_VERSION),9)"
differentiated only between V9 and non-V9, which it assumed to be V8.
Howerver, now, we have V10 while V11 is being tested, all of which would
be treated like V8, which is incorrect.
To address this issue the compiler version classification is now done with
     ifeq ($(shell expr $(COMP_VERSION) \>= 9), 1)
            CCE_GE_V9 := TRUE
     else
            CCE_GE_V9 := FALSE
     endif
and it is tested with
     ifeq ($(CCE_GE_V9),TRUE)

## Checklist

The proposed changes:
- [X] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
AMReX-Codes#1265)

…d 'hostname -f' command to 'hostname'

## Summary
updated Make.machines:
1. added poplar and redwood to frontier-coe machines;
2. changed name string matching for frontier-coe to only
    yield exact matches (filter); hence, !="" means exact match;
3. changed `hostname -f` command (which includes the domain) to `hostname` to
    avoid false negatives, e.g., not recognizing `hostname -f`==tulip.cm.cluster as tulip;

## Additional background

## Checklist

The proposed changes:
- [x] fix a bug or incorrect behavior in AMReX
- [x] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
AMReX-Codes#1270)

AMReX-Codes#1168  Summary
Changed Tools/GNUMake/sites/Make.frontier-coe to
1. add support for additional AMD GPU SKUs;
2. include poplar and redwood in the list of frontier-coe machines

## Additional background
Changed
   ifeq ($(which_computer),$(filter $(which_computer),tulip))
     ifeq ($(USE_HIP),TRUE)
       CXXFLAGS += --amdgpu-target=gfx906
       HIPCC_FLAGS += --amdgpu-target=gfx906
     endif
   endif
to
   ifeq (,$(filter $(which_computer),poplar redwood tulip))
     $(error Unknown Frontier CoE computer, $(which_computer))
   else
     ifeq ($(USE_HIP),TRUE)
       CXXFLAGS += --amdgpu-target=gfx906,gfx908
       HIPCC_FLAGS += --amdgpu-target=gfx906,gfx908
     endif
   endif
For AMD GPUs, the compile target should be exact to avoid runtime errors,
i.e., one should generally not assume that a binary targeting an older GPU
will always run correctly on a newer GPU.  However, building "fat binaries"
is possible, so that one does, typically, not need one separate binary
specifically built for each AMD GPU SKU available on the platform.



## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
## Summary

Due to a compiler bug (I believe), we have to modify amrex::Math::abs for
beta8 on DG1.

## Additional background

After spending many hours debugging Tutorial/Amr/Adevection_AmrCore/
on DG1, it is found that `MultiFab::norm0` does not return the correct
result, because of the use of `amrex::Math::abs`.  After making the change
in this PR, the test now produces correct results with AMR.

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
Remove redundant MPI_Allreduce and cleanup.
Missing Gpu::synnhronize() call after host to device memcpy.
## Summary
ensure host_name_short is always defined in Tools/GNUMake/Make.machines

## Additional background

## Checklist

The proposed changes:
- [x] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
# Summary

TagBoxArray functions can now run on GPU now.  We no longer rely on unified memory for TagBoxArray functions.

## Additional background

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [x] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
In AMReX-Codes#1258, TagBoxArray::numtags was removed.  However, IAMR still needs it.
So a new function, hasTags, is added for IAMR.
WeiqunZhang and others added 28 commits November 4, 2020 17:00
## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [x] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
* Doc: move ENABLE_CUDA_FASTMATH to the GPU options section

* CMake: change all options to include AMReX_ namespace

* CMake: remove ENABLE_ACC option

* CMake: make AMReX_SPACEDIM a multi-valued string

* Doc: add option AMReX_ENABLE_TESTS

* CMake: rename AMReX_Options.cmake to AMReXOptions.cmake

* CMake: refactor GPU-related options

* CMake: MPI must be turned off if DPCPP is enabled

* CMake: set via precision via selection of value

* CMake: forgot to update Particles/CMakeLists.txt

* CMake: append prefix to TL_PROFILE

* Update Docs/sphinx_documentation/source/AMReX_Profiling_Tools.rst

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

* Update Docs/sphinx_documentation/source/AMReX_Profiling_Tools.rst

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

* Update Docs/sphinx_documentation/source/BuildingAMReX.rst

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

* CMake: components in Config file shall not have namespace prefix

* Revert "CMake: components in Config file shall not have namespace prefix"

This reverts commit 924cf15.

* Update Tools/CMake/AMReX_SetupCUDA.cmake

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

* CMake: update Doc

* Update Tools/CMake/AMReX_SetupCUDA.cmake

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

Co-authored-by: Axel Huebl <axel.huebl@plasma.ninja>
Fix the variable being checked to enable HYPRE.


## Checklist

The proposed changes:
- [X] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
## Summary
DPC++ does not have the concept of null stream.  Adding a sync after
htod_memcpy on the "null" stream will help eliminate potential bugs (e.g.,
htod_memcpy is called before MFIter that uses non-null stream).

## Additional background

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
…eX-Codes#1518)

HypreNodeLap class creates the hypre IJ instance in its constructor. Therefore,
the custom option namespace specified by user must be part of the constructor
arguments.
## Summary
`make clean` now does what `make realclean` does.  `make realclean` is kept.
`make cleanconfig` is introduced to do what `make clean` used to do.  For
most users, `make clean` is the one that should be used.

Also add some tests in makefiles to avoid error messages in case when `make
clean` is run on a makefile with the default not suitable for the system.

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
## Summary

Make the _environment variable_ that sets a default CUDA architecture all-caps, as this is way more common in Unix.

Typical Values: `7.0` or `Volta` (i.e. for V100)

## Additional background

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
## Summary
* Implement atomicAdd for CUDA arch < 6.0.

* Remove the CUDA arch check in CMake.

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [x] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
## Summary

* Remove summitdev and peak.

* Environment variable OLCF_MODULEPATH_ROOT is used in addition to host name
  to detect OLCF machines.

## Additional background

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
## Summary
This fixes a one-time memory leak in MPI_Datatypes defined by AMReX as reported in AMReX-Codes#1525.

## Checklist

The proposed changes:
- [x] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
## Summary
Add assertion to catch nested MFIters (e.g., MultiFab functions are called inside MFIter).

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [x] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
`FillPatchIterator` is derived from `MFIter`.  We need to reset
`MFIter::depth` so that the ctor of `FillPatchIterator` can start `MFIter`.
* Use atomics instead of volatile to read status written to global memory by
  other blocks

* Workaround `Random()` bug in the scan test

* Limit the memory usage in the scan test
…1534)

## Summary
adds documentation of hypre.adjust_singular_matrix
## Additional background

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [X] are described in the proposed changes to the AMReX documentation, if appropriate
Access of particle variable names with a bound-check, so users
definitely pass the right amount of names.
## Summary
`MultiFab::setVal` should not be called inside `MFIter`.

## Additional background
This was caught by AMReX-Codes#1530.

## Checklist

The proposed changes:
- [x] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
## Summary
No longer set CUDA device cache configuration to prefer L1 cache.  This does
not appear to affect any kernels in a negative way.  With more shared memory
available, reduction functions are faster in some cases.

## Additional background

## Checklist

The proposed changes:
- [ ] fix a bug or incorrect behavior in AMReX
- [ ] add new capabilities to AMReX
- [ ] changes answers in the test suite to more than roundoff level
- [ ] are likely to significantly affect the results of downstream AMReX users
- [ ] are described in the proposed changes to the AMReX documentation, if appropriate
This PR adds a command line option to `fcompare` that takes in a user-specified tolerance for absolute error and adds logic to compare both absolute and relative errors against user-specified tolerances.

Since `-a` is already used as a short-form for `--allow_diff_grids`, I have only used `--abs_tol` as the option without a short version when parsing command line options.
PR AMReX-Codes#1537 introduced the option to allow tolerance checks on both absolute and
relative tolerances. However, the check used `and` instead of `or` to allow
tests to pass when either absolute or relative error was below user-specified
tolerance.
@sayerhs sayerhs closed this Nov 14, 2020
@sayerhs sayerhs deleted the d/fcompare-fix branch December 2, 2020 13:27
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.