Previously, when you got a host/control portal from
`ArrayHandleVirtual`, you got a version of an `ArrayPortal` that manged
its own reference to the virtual structure that was implementing the
portal. This was not done for device/execution portals because those
objects could not do the appropriate resource management from the
execution environment.
Rather than releasing the host object to the portal, keep the host
object managed by `StorageVirtual`. This allows the control and
execution portals to be the same type, which we will need to be friendly
with new array objects.
1. The code now works without CUDA_LAUNCH_BLOCKING set by using explicit
synchronizations where required.
2. The code has also been modified to use thread specific memory spaces,
which for Kokkos' Cuda backend means per thread streams.
fb130d42d Remove test for ArrayHandleCompositeVector with bad lengths
7cbf40d5b Back out of using a composite of transform arrays
7ff1a690d Support ArrayHandleCompositeVector with 1 component
5391e353a Convert ArrayHandleExtractComponent to new buffer-style array
22fac15b3 Convert ArrayHandleCompositeVector to new buffer-style array
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Robert Maynard <robert.maynard@kitware.com>
Merge-request: !2330
3c50a7ea4 Fix assert for pathlines. Add example from manual that was failing.
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Kenneth Moreland <kmorel@sandia.gov>
Merge-request: !2335
As we update the `ArrayHandle`s to the new buffer style, simplify
`ArrayHandleSwizzle` by implementing it on top of
`ArrayHandleTransform`. Also, introduce some C++11 template concepts to
handle variable arguments better.
In this conversion, some features have been removed. Previously if the
swizzle dropped some components, you were still allowed to write to the
array. This would be an in place write that would only update the passed
components. This feature has been removed and if any components are
dropped, the array becomes read only. If all components are swizzled,
then the array becomes writable (through templating).
Another lost feature is that the swizzle map is no longer checked for
correctness. The array handle just assumes that the indices are correct.
It should be noted that it does not look like `ArrayHandleSwizzle` is
actually used anywhere. If it is not used by any customers, we may
want to consider deprecating the class.
The old version of `ArrayHandleCompositeVector` would check (at
construction) to make sure that all the arrays had the same length. This
check has been removed because (1) it did not always catch all problems
(such as if the input arrays change), (2) debug builds check for out-of-
range array accesses anyway, and (3) it required a lot of extra template
expansion.
Because that check has been removed, also remove the part of the
`ArrayHandleCompositeVector` test that checks this check.
When `ArrayHandleCompositeVector` has only 1 component, it is supposed
to have a special template that uses the base value type rather than a
`Vec` of that type. However, the `Storage` with the value type was
missing. I'm not sure how we weren't getting compile errors before, but
moving to the new buffer arrays seems to bring about the expected error.
The `StorageImplicit.h` header is no longer used. However, just removing
it causes compiles to fail if they include it. To make the transition
better, re-add this file, but cause a deprecation warning if it is
included.
`vtkm::[U]Int*` types depending of each platform/arch would translate to
different integer types. We cannot make a generalization with this.
A simple way to handle this is to simply use primitive types, such as
char, signed char, long..., instead of vtkm::[U]Int*. This makes sure
that every primitive type is actually instantiated.
We could potentially use this in other filters instantiation file.
This error has been reported in VTK when being build with vtkm.
It should be noted that a break in backward compatibility is introduced.
The implementation class passed to `ArrayHandleDecorator` changed the
specification of `AllocateSourceArrays`. Thus, decorators that were
previously allocatable now no longer will be until that method is
updated to the new form.
A large portion of the VTK-m filters are now compiled into the
vtkm_filter library. These pre-built filters now don't include
the respective hxx file to remove the amount of template
instantiation users do.
To verify that this removal reduces compiler memory ( and maybe time)
I profiled the compiler while it building filter tests in debug mode.
Here is a selection of results:
```
CleanGrid 10.25s => 9.01s, 901MB => 795MB
ExternalFaces 13.40s => 5.96s, 1122MB => 744MB
ExtractStructured 4.69s => 4.75s, 492MB => 492MB
GradientExplicit 22.97s => 5.88s, 1296MB => 740MB
```
The array given to `ArrayHandleGroupVec` should really have a number of
values that divides evenly among the `Vec`s. Previously, if this was not
the case, an error exception was thrown. These changes allow this
condition. Extra components are dropped and a warning is logged.
Most `ArrayHandle` implementations must create an
`std::vector<vtkm::cont::internal::Buffer>` object. It is most helpful
to create it in the constructor when initializing the superclass (so the
superclass does not have to create it's own). Added a `CreateBuffers`
convenience function to make it easy to build these vectors.
MR !2311 fixed an issue where some base C types were not recognized by
`SerializableTypeString`. However, the fix was such that different types
with the same layout had the same string. For example `char` and
`signed char` both were given the string `I8`. That meant that the
serialization/deseralization would work, but the deserialization could
change the type. That could cause problems if two arrays were expected
to have the same type but did not.
This change undoes much of MR !2311 and redoes it so that the types are
correct.
C++ template considers some types different even though they have the
exact same format. For example `int`, `long`, and `long long` all match
different types even though they all represent either signed 32-bit ints
or signed 64-bit ints.
List all these possible types in
`TypeListScalarAll`.
ec4a434dc Include all int types in SerializableTypeString
d9fe6a546 Fix ArrayHandleCast serializable type name
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Sujin Philip <sujin.philip@kitware.com>
Merge-request: !2311
- It explicitly instantiate its concrete templates to
the supported types.
- It disables implicit template instantations for the
the supported types.
- Also fix small typo in Contour impl include guards
Signed-off-by: Vicente Adolfo Bolea Sanchez <vicente.bolea@kitware.com>
Previously, `SerializableTypeString` was specialized only for the VTK-m
defined int types (`Int8`, `UInt8`, `Int16`, etc.). You would think this
would cover all the base int types, but in fact C++ defines multiple int
types that have the exact same bit layout. These duplicates will not
match each other when matching template specializations. For example,
`char` is either the same as `signed char` or `unsigned char`, but will
match neither. Likewise, `long` is the same as either `int` or `long
long`, but all these types will match different template
specializations.
Now all integer types are covered.
The specialization for SerializableTypeString for ArrayHandleCast tried
to get the serialization for the base type and the storage tag. This is
wrong since the storage tag itself is not seralizable nor has a type
string. Instead, just record the type name for the sub ArrayHandle
itself.
Previously you had to create a special virtual object to place in the
`Buffer`'s metadata. This added a lot of difficulty that is probably
unnecessary.
Instead, just have `Buffer` hold an arbitrary object and some simple
functions to copy and delete it. There may be issues with type safety
across translation units, but we'll deal with that when/if we run into
it.
ArrayRangeCompute.hxx was removed, but other code may be including it.
The file was replaced. It now just includes its replacement
(ArrayRangeComputeTemplate.h) and forces a deprecation warning.
e3dfa4891 Do not attempt to move non-trivial objects in Variant
8e11b3ecd Remove ArrayPortalCheck
21db210a7 Make separate exec and cont versions of Variant
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Robert Maynard <robert.maynard@kitware.com>
Merge-request: !2274
The `Variant` class has separate implementations for its move and copy
constructors/assignment operators depending on whether the classes it
holds can be trivially moved. If the objects are trivial, Variant is
trivial as well. However, in the case where the objects are not trivial,
special construction and copying needs to be done.
Previously, the non-trivial `Variant` defined a move constructor that
did a byte copy of the contained object and reset the right hand side
object so that it did not attempt to destroy the object. That usually
works because it guarantees that only one version of the `Variant` will
attempt to destroy the object and its resources should be cleaned up
correctly.
But C++ is a funny language that lets you do weird things. Turns out
there are cases where moving the location of memory for an object
without calling the proper copy method can invalidate the object. For
example, if the object holds a pointer to one of its own members, that
pointer will become invalid. Also, if it points to something that points
back, then the object will need to update those pointers when it is
moved. GCC's version of `std::string` seems to be a type like this.
Solve the problem by simply deleting the move constructors. The copy
constructors and destructor will be called instead to properly manage
the object. A test for these conditions is added to `UnitTestVariant`.
This portal only works on the control environment, which means it cannot
work with the new `ArrayHandle` type. Recent changes to
`ArrayHandleMultiplexer` also do not allow this, so just remove it
rather than try to fix it.
The `Variant` class is templated to hold objects of other types.
Depending on whether those objects of are meant to be used in the
control or execution side, the methods on `Variant` might need to be
declared with (or without) special modifiers. We can sometimes try to
compile the `Variant` methods for both host and device and ask the
device compiler to ignore incompatibilities, but that does not always
work.
To get around that, create two different implementations of `Variant`.
Their API and implementation is exactly the same except one declares its
methods with `VTKM_CONT` and the other its methods `VTKM_EXEC`.
Instead, always use precompiled versions of range computing. This means
you won't be able to specify the type. Currently, types are limited to
scalars vecs up to size 4.
The main motivation for this change is to allow you to include Field.h
with a non-device compiler. This is an important feature for our
customers.
I plan in the future to implement a mechanism to pull out a component of
most ArrayHandle's as a single array. This would enable us to support a
precompiled version that can compute the range of arbitrarily sized
Vecs.
0b2ddb83b UnitTestBounds custom compile flags expressed via generator expression
1ed5dfca0 vtkm/Math frexp(float) version works now with HIP
ff381bf8b vtkm/Swap works with hip
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Kenneth Moreland <kmorel@sandia.gov>
Merge-request: !2301
0b1c48a3e Identify number of partitions in filter execution
b33c54bf6 Add ScheduleTask to performance log
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Robert Maynard <robert.maynard@kitware.com>
Merge-request: !2302
When you execute a filter, the default behavior is to do the execution
on each partition of the data set independently. This code path is
followed even for non-partitioned data; the `DataSet` is wrapped in a
`PartitionedDataSet` of one partition.
Make performance logging a bit more clear by only giving one scoped log
for a basic `DataSet` and recording the number of partitions executed in
the log.
When `DeviceAdapterAlgorithm::ScheduleTask` was called directly (i.e.
not through `Schedule`), nothing was added to the log. Adding
`VTKM_LOG_SCOPE` to these methods so that all scheduling is added to the
performance log.
f66dc780c Update ArrayHandleImplicit to new array style with Buffer
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Robert Maynard <robert.maynard@kitware.com>
Merge-request: !2246
- It fixes a lingering error triggered with BenchContour
- It reenables BenchContour
Signed-off-by: Vicente Adolfo Bolea Sanchez <vicente.bolea@kitware.com>
The old atomic compare and swap operations (`vtkm::AtomicCompareAndSwap`
and `vtkm::exec::AtomicArrayExecutionObject::CompareAndSwap`) had an
order of arguments that was confusing. The order of the arguments was
shared pointer (or index), desired value, expected value. Most people
probably assume expected value comes before desired value. And this
order conflicts with the order in the `std` methods, GCC atomics, and
Kokkos.
Change the interface of atomic operations to be patterned off the
`std::atomic_compare_exchange` and `std::atomic<T>::compare_exchange`
methods. First, these methods have a more intuitive order of parameters
(shared pointer, expected, desired). Second, rather than take a value
for the expected and return the actual old value, they take a pointer to
the expected value (or reference in `AtomicArrayExecutionObject`) and
modify this value in the case that it does not match the actual value.
This makes it harder to mix up the expected and desired parameters.
Also, because the methods return a bool indicating whether the value was
changed, there is an additional benefit that compare-exchange loops are
implemented easier.
For example, consider you want to apply the function `MyOp` on a
`sharedValue` atomically. With the old interface, you would have to do
something like this.
```cpp
T oldValue;
T newValue;
do
{
oldValue = *sharedValue;
newValue = MyOp(oldValue);
} while (vtkm::AtomicCompareAndSwap(sharedValue, newValue, oldValue) != oldValue);
```
With the new interface, this is simplfied to this.
```cpp
T oldValue = *sharedValue;
while (!vtkm::AtomicCompareExchange(sharedValue, &oldValue, MyOp(oldValue));
```
The `ReleaseResourcesExecution` method makes changes by booting data off
of the execution environment. But logically the array does not change.
It remains the same size with the same contents. Thus, it makes sense
for this to be a const method.
Also modified some deprecated methods a bit to remove some unnecessary
templates.
The `UnitTestParticleAdvectionFilter` was only testing advection in
structured cell sets (with either uniform or rectilinear points). This
did not test all paths (i.e. grid evaluators and locators) of particle
advection. Changed the test to also try grids with explicit cells.
Note that the explicit cells are created by just converting the
rectilinear grids to explicit grids. It would be better to get at least
one small dataset that starts as unstructured.
f5c5b6188 Log the computation of ranges in fields and arrays
bef55f8ee Log the conversion of num components to offsets
420b9d397 Log the building of arrays for scatters and masks
Acked-by: Kitware Robot <kwrobot@kitware.com>
Merge-request: !2283
c26e193fe Use kokkos provided routines for sort and copy
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Robert Maynard <robert.maynard@kitware.com>
Merge-request: !2277
Previously vtk-m allowed users to issue atomic loads on constant
values which is problematic for the following reasons:
- can be a source of undefined behavior
- not supported by kokkos
This issue was detected when using kokkos HIP atomic implementation
When you invoke a worklet, a scoped performance log is automatically
generated. The previous version gave the name of the worklet, which is
technically everything you need to know. However, it is also convenient
to know what type of worklet it is (e.g. a field map worklet or a
topology map worklet). This can be determined by looking at the
definition of the worklet in the source code, but that is not practical
if, for example, you want to perform automated analysis.
This change prints out the type of the dispatcher instead of the
worklet. The full type of the dispatcher includes the worklet as a
template parameter, so you get both the dispatcher/worklet type and the
worklet itself. So previously you would get log lines like
```
Invoking Worklet: 'vtkm::worklet::vtkm::worklet::ExternalFaces::NumFacesPerCell'
```
They now look like this:
```
Invoking Worklet: 'vtkm::worklet::DispatcherMapTopology<vtkm::worklet::ExternalFaces::NumFacesPerCell>'
```
268a07bb0 remove test on large graph and sample data
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Kenneth Moreland <kmorel@sandia.gov>
Merge-request: !2278
- It also remove termination on bench error
- It disables passing fields in Countour to skip an error.
Signed-off-by: Vicente Adolfo Bolea Sanchez <vicente.bolea@kitware.com>
932f8b82f Merge branch 'concurrent_union_find' of gitlab.kitware.com:ollielo/vtk-m into concurrent_union_find
eaff27195 minor changes on comments on findRoot()
9fef540ba minor changes on comments on findRoot()
916661e33 Merge branch 'concurrent_union_find' of gitlab.kitware.com:ollielo/vtk-m into concurrent_union_find
a036b0f98 Minor change based on code review.
9e68022ac Minor change based on code review.
1185f70bc Data Race Resolution
aed7e02da Merge branch 'concurrent_union_find' of gitlab.kitware.com:ollielo/vtk-m into concurrent_union_find
...
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Kenneth Moreland <kmorel@sandia.gov>
Merge-request: !2248
Although `vtkm::internal::Variant` respected the trivially copyable
attribute of the types it contains, it was never totally trivial (i.e.
`std::is_trivial<Variant<...>>` was never true). The reason was that
`Variant` was initializing its `Index` parameter to signify that it was
not initialized. However, the fact that `Index` was initialized meant
that it was not trivially constructed.
Now, `Variant` type checks its types to see if they are all trivially
constructible. If so, it makes itself trivially constructible.
This means that `Index` may or may not be valid if `Variant` is
constructed without an argument. This in turn means that the result of
`Variant::IsValid` becomes undefined. That should be OK in practice.
`Index` will "point" to an uninitialized object, but that object is
trivially constructed anyway. However, that could cause problems if
developers used `IsValid` to determine if something is selected.
The new name reflects better what the underlying algorithm does. It also
helps prevent confusion about what types of data the locator is good
for. The old name suggested it only worked for structured grids, which
is not the case.
We would really like to be able to include `vtkm::cont::ColorTable` in
such a way that you don't have to compile device code (unless you are
actually compiling functions for the device). Thus, the `Map` functions
of `ColorTable` were in a special `ColorTable.hxx` that contains the
"implementation" for `ColorTable`.
That is confusing to many users. It is more clear to simply have `.h`
headers that do a specific thing. To achieve these two goals, the `Map`
functionality of `ColorTable` is separated out into its own header file.
So you don't need to be using a device compiler just to use `ColorTable`
(including `ColorTable.h`), but you do need to use a device compiler if
mapping values to colors (including `ColorTableMap.h`).
6a3ba4291 Fix warning about unused function
c6a4f9b79 Fix warning about return value
9465d2611 Adjust TransferFunction worklet to get color table as ExecObj argument
11996f133 Remove virtual methods from ColorTable
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Robert Maynard <robert.maynard@kitware.com>
Merge-request: !2261
Previously, the `TransferFunction` worklet required you to call
`PrepareForExecution` on the `ColorTable` and give that to the
constructor. Bad things can happen if the `ColorTable` used in the
constructor does not match the device the worklet actually gets invoked
on. Change this to make the `ColorTable` a worklet argument of type
`ExecObj`. The dispatcher will automatically call
`ColorTable::PrepareForInput` and guarantee that the devices match.
Virtual methods are being deprecated, so remove their use from the
ColorTable classes. Instead of using a virtual method to look up a value
in the ColorTable, we essentially use a switch statement. This change
also simplified the code quite a bit.
The execution object used to use pointers to handle the virtual objects.
That is no longer necessary, so a simple `vtkm::exec::ColorTable` is
returned for execution objects. (Note that this `ColorTable` contains
pointers that are specific for the particular device.) This is a non-
backward compabible change. However, the only place (outside of the
`ColorTable` implementation itself) was a single worklet for converting
scalars to colors (`vtkm::worklet::colorconversion::TransferFunction`).
This is unlikely to affect anyone.
I also "fixed" some names in enum structs. There has been some
inconsistencies in VTK-m on whether items in an enum struct are
capitolized or camel case. We seem to moving toward camel case, so
deprecate some old names.
482266b44 Particle: explicitly make the copy ctor and assignment op
1037aa756 AssignerPartitionedDataSet: mark dtor as override
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Robert Maynard <robert.maynard@kitware.com>
Merge-request: !2259
One of the features of `UnknownArrayHandle` is that it allows you to
query how many `Vec` components each value has without resolve the type
of the array. The functionality to implement this failed if you tried to
store an `ArrayHandle` that stored `Vec`-like objects with `Vec` sizes
that varied from value to value (i.e. an `ArrayHandleGroupVecVariable`).
Storing such an array in `UnknownArrayHandle` might not be the best
idea, but it should probably work. This change allows you to store such
an array. If you try to query the number of components, you will get 0.
e706880d7 Fix unnecessary deprecation warnings in visual studio
f7cc03107 Fix deprecated warnings
b27e4c7ea Ignore files for deprecated virtual classes for SourceInInstall test
284774cf4 Turn on "no virtual" option on some CI builds
cd08fd499 Add changelog for removing virtual methods
63ef84ed7 Optionally remove all use of ArrayHandleVirtual
e11f612ad Deprecate ArrayHandleVirtual
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Robert Maynard <robert.maynard@kitware.com>
Merge-request: !2256
As we remove more and more virtual methods from VTK-m, I expect several
users will be interested in completely removing them from the build for
several reasons.
1. They may be compiling for hardware that does not support virtual
methods.
2. They may need to compile for CUDA but need shared libraries.
3. It should go a bit faster.
To enable this, a CMake option named `VTKm_NO_DEPRECATED_VIRTUAL` is
added. It defaults to `OFF`. But when it is `ON`, none of the code that
both uses virtuals and is deprecated will be built.
Currently, only `ArrayHandleVirtual` is deprecated, so the rest of the
virtual classes will still be built. As we move forward, more will be
removed until all virtual method functionality is removed.
386301719 Test UnknownArrayHandle behavior on special arrays
872da1f8e Suppress unnecessary deprecation warnings on VS
bb443bbc2 Replace the implementation of VariantArrayHandle
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Robert Maynard <robert.maynard@kitware.com>
Merge-request: !2255
`UnknownArrayHandle` has special behavior when putting in or getting out
an `ArrayHandleMultiplexer` or an `ArrayHandleCast`. When putting in
either of these, `UnknownArrayHandle` gets the actual array stored in
the multiplexer and cast so that you can later retrieve the base array.
Likewise, when you get the array out with `AsArrayHandle`, you can give
it an `ArrayHandleCast` or `ArrayHandleMultiplexer`, and you will get
the base array placed inside of it.
The Visual Studio compiler has an annoying habit where if you use a
templated class or method with a deprecated class as a template
parameter, you will get a deprecation warning where that class is used
in the templated thing. Thus, if you want to suppress the warning, you
have to supress every instance of the template, not just where the
template is declared.
This is annoying behavior that is thankfully not replicated in other
compilers.
The implementation of `VariantArrayHandle` has been changed to be a
relatively trivial subclass of `UnknownArrayHandle`.
The advantage of this change is twofold. First, it removes
`VariantArrayHandle`'s dependence on `ArrayHandleVirtual`, which gets us
much closer to deprecating that class. Second, it ensures that
`UnknownArrayHandle` is a reasonable replacement for
`VariantArrayHandle`, so we can move forward with replacing that.
C++11 introduced the `std::vector::data()` method. In addition to being
more syntatically pleasing, it should correctly handle the condition
when the `std::vector` is size 0 and therefore has no real pointer.
(Expressions like `&v[0]` are undefined when the `vector` is empty.)
The new method `ArrayHandle::DeepCopy` had the pattern such that the
data in the `this` array was pushed to the provided destination array.
However, this is the opposite pattern used in the equivalent method in
VTK, which takes the data from the provided array and copies it to
`this` array.
So, swap the pattern to match that of VTK. Also, make the method name
more descriptive by renaming it to `DeepCopyFrom`. Hopefully, users will
read that to mean given the `ArrayHandle`, you copy data from the other
provided `ArrayHandle`.
A recent modification to `ArrayCopy` created a fast path for when
copying arrays of the same type. This fast path just deep copies the
buffers. The issue was that the buffer copy was creating new buffers
instead of updating the existing buffers. The passed in `ArrayHandle`
would get updated to the new buffers, but any other `ArrayHandle`s
sharing those buffers would still have the old versions. That would lead
to unexpected errors in code like this.
```cpp
vtkm::cont::ArrayHandle<T> outArray1;
vtkm::cont::ArrayHandle<T> outArray2 = outArray1;
vtkm::cont::ArrayCopy(inArray, outArray2);
```
If `inArray` was a different type than `outArray2`, then the data for
both `outArray1` and `outArray2` would be updated (which is the expected
behavior for something considered a "pointer"). However, if `inArray`
was the same type as `outArray2`, then the fast path would change
`outArray2` but leave `outArray1` the same.
This behavior has been corrected so that, in this case, the data of
`outArray1` always follows that of `outArray2`.
The parallel sorts in the device adapter are documented to not be
stable. Up until recently, the sorts for all the supported devices
happened to be stable (or at least provided a stable sort where needed).
However, the recent Kokkos adapter provides a sort that is no stable.
This broke some tests that relied on stable sorts when they should not
have.
Fix the tests that relied on stable sort to check the results.
