Fill device_matrix_data #1683
Fill device_matrix_data #1683
Conversation
|
This would also mean to change the constructor (which is more important to me). The alternative would be to introduce an enum for the two different initialization modes (i.e. |
0893e50 to
1c286a1
Compare
|
Does it need to happen in a single function call though? Can't we ask the user to call |
|
For me the constructor is more important than the fill method. Users usually know when they create the device matrix data if it's going to be used in an assembly setting or not. For the assembly, I assume that they almost always want to zero out the data, because otherwise |
|
|
||
| /** |
|
Why does it need to be done in a single step? Can't it be two steps (construction + fill)? Can you describe a matrix assembly scenario where the number of values to be written isn't known beforehand? And if so, couldn't it be more efficient to compute the storage requirements beforehand and leave no gaps? |
|
@upsj @yhmtsai it is pretty common in FEM applications to not use the total number of DOFs, but instead use |
|
@upsj doing it in one step is syntactic sugar. But so is setting the dimension. The one step is more convenient, so it might be easier for the user, both in terms of slightly less code, and discoverbility. Right now, do our users know that they will get uninitialized memory? I would not count on it. |
|
I know the common approach of assembling elements independently (which is what I built sum_duplicates for), but what I'm not familiar with is the case where part of the preallocated storage is left unused. Is that related to boundary conditions? It should be straightforward for users to fix this in their own assembly routines (just set the values to 0), but I'm not opposed fundamentally to it, just want to make it an explicit operation. On the syntactic sugar side, I slightly disagree: There are unchangeable properties of the |
|
Used storage can appear in FVM or FDM on the boundary. From the user perspective, it should not matter if they explicitly set those to zero or just skip them. In some cases, setting the values might seem a bit odd, because it would involve invalid non-zero coordinates. where |
|
The question there is: Is the resulting row/column then zero itself? Because that would make the matrix singular, and maybe bring us back to #842 if we want to use it in solvers. Do we need some post-processing functionality for that use case as well? |
|
Not in my example, since |
|
I see, that makes sense. But that would not work in a more generic grid, where the indexing isn't as simple. But yes, let's not get too much off track. What I'm saying is: All of our matrix data structures do not initialize their arrays, so that is an expectation that would already be formed in other parts of the code. But again, I am not opposed to it, I just want to suggest that we do it everywhere, if we do it. In that case, |
|
I agree that if we accept this PR, it should be added throughout Ginkgo. But IMO, this should be done in subsequent PRs, to keep the changes in each one short. I would also leave |
|
The take-away from the discussion in today's meeting:
|
1c286a1 to
cb521dd
Compare
|
I changed this PR to only add the |
cb521dd to
bda16f7
Compare
bda16f7 to
d362f5b
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #1683 +/- ##
===========================================
- Coverage 90.24% 89.86% -0.38%
===========================================
Files 764 764
Lines 62913 62934 +21
===========================================
- Hits 56773 56555 -218
- Misses 6140 6379 +239 ☔ View full report in Codecov by Sentry. |
The main use case is in combination with `sum_duplicates` and `remove_zeros` to simplify the assembly setup.
d362f5b to
02ce1a8
Compare
|
This merge allows to fill the entries of a device_matrix_data with zeros. The main use case is in combination with `sum_duplicates` and `remove_zeros` to simplify the assembly setup. Related PR: ginkgo-project#1683
This PR allows to fill the entries of a device_matrix_data with a specified value. The main use case is in combination with
sum_duplicatesandremove_zerosto simplify the assembly setup.