TLS for VectorPostprocessors #7819

permcody · 2016-10-05T21:03:05Z

After this is merged we can go through the existing samplers and remove temporary vectors
or other workarounds to not having TLS.

refs #7809

Ping @friedmud, @dschwen

aeslaughter · 2016-10-06T13:48:47Z

framework/src/vectorpostprocessors/VectorPostprocessor.C

@@ -47,5 +48,8 @@ VectorPostprocessor::getVector(const std::string & vector_name)
 VectorPostprocessorValue &
 VectorPostprocessor::declareVector(const std::string & vector_name)
 {
-  return _vpp_fe_problem->declareVectorPostprocessorVector(_vpp_name, vector_name);
+  if (_vpp_tid)
+    return _thread_local_vector;


How are you envisioning these being used and joined by users?

Exactly as they are in UserObjects and Postprocessors. We already have a threadJoin() routine that's being called on these objects. The content of these extra vectors is not important. It's just there as scratch space as the VectorPostprocessor executes.

There needs to be one local vector for each vector_name, right?

friedmud · 2016-10-06T15:55:00Z

framework/src/vectorpostprocessors/VectorPostprocessor.C

@@ -47,5 +48,8 @@ VectorPostprocessor::getVector(const std::string & vector_name)
 VectorPostprocessorValue &
 VectorPostprocessor::declareVector(const std::string & vector_name)
 {
-  return _vpp_fe_problem->declareVectorPostprocessorVector(_vpp_name, vector_name);
+  if (_vpp_tid)
+    return _thread_local_vector;


There needs to be one local vector for each vector_name, right?

friedmud · 2016-10-06T15:56:10Z

Also: there should be a test or two showing how to use the new functionality.

permcody · 2016-10-06T15:57:08Z

Yes - we definitely do need multiple vectors. I'll also add a test object.

permcody · 2016-10-07T21:23:20Z

So I decided not to add a dummy test object for this. I reverted Daniel's "fix" instead so we have a real object and a real test in the framework already with the Histogram VPP. Also, I rewrote SamplerBase to get rid of all the temporaries so we can verify that this all works with threads properly.

dschwen · 2016-10-07T21:33:48Z

The revert of the fix also stripped the overrides off

permcody · 2016-10-07T21:36:07Z

Well you can't have your cake and eat it too, but apparently you "can has" all the donuts you want (and do want).

I'll add those back into the separate fixup commit that already sits on top of the revert commit and repush.

dschwen · 2016-10-11T09:27:02Z

framework/include/vectorpostprocessors/VectorPostprocessorData.h

-  /// Values of the post-processors at the time t-1
-  std::map<std::string, std::map<std::string, VectorPostprocessorValue*> > _values_old;
+  std::set<std::string> _requested_items;
+  std::set<std::string> _supplied_items;
 };


Is this for a planned dependency resolution?

Yes and no. Right now there's already dependency resolution on the objects themselves because they are UserObjects and sorted in the warehouse. The intention for this was to add in a new sanity check for the vectors themselves if you couple to vectors, there is no check on whether a vector that you couple to exists right now. I started on this but need to flesh it out quite a bit more and want to unify the way it works between VectorPostprocessors and Postprocessors. I can either continue to work on this or continue that work in another PR. I kind of want to handle it separately since this is already getting to be a substantial PR.

friedmud · 2016-10-11T18:01:36Z

@permcody BTW: I'm somewhat abusing the fact that there is no sanity checking on VPP vector declarations for my own purposes.

Check out: https://github.com/friedmud/mocodile/blob/devel/src/vectorpostprocessors/RayTracingResultsVectorPostprocessor.C

I don't declare the vectors until initialize(). That's because I don't actually know what they are until that point (this is outputting a std::map<std::vector> that is built elsewhere during execution).

Please don't undo my ability to do that 😉

permcody · 2016-10-11T18:41:42Z

Ha! Well your "we allow that" may have to take a back seat to having MOOSE do something intelligent for our users. We can still support late creation of vectors with additional Actions or something. At some point though we really should be able to check the sanity of the system before beginning execution. I already got bit on this on one of my first uses of VectorPostprocessors since I renamed my vector during development and my coupled object worked just fine and drove me mad while I tried to figure out while I only had zeros in my vector (or nothing at all).

BTW - @YaqiWang abuses Postprocessors in a similar fashion so I don't have satisfactory checks like I want to.

I'm supportive of late declarations, we might just need to design that in to the system so that we can still check them.

friedmud · 2016-10-12T02:28:38Z

Sounds good. I was just throwing the idea out there. Since there aren't
that many users of VPP's yet (but it is growing) we need to make sure to
cover our use-cases as development continues...

On Tue, Oct 11, 2016 at 2:41 PM Cody Permann notifications@github.com
wrote:

Ha! Well your "we allow that" may have to take a back seat to having MOOSE
do something intelligent for our users. We can still support late creation
of vectors with additional Actions or something. At some point though we
really should be able to check the sanity of the system before beginning
execution. I already got bit on this on one of my first uses of
VectorPostprocessors since I renamed my vector during development and my
coupled object worked just fine and drove me mad while I tried to figure
out while I only had zeros in my vector (or nothing at all).

BTW - @YaqiWang https://github.com/YaqiWang abuses Postprocessors in a
similar fashion so I don't have satisfactory checks like I want to.

I'm supportive of late declarations, we might just need to design that in
to the system so that we can still check them.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7819 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AA1JMTd2X5zhClphcRb5lNG-LQCdIJSjks5qy9hngaJpZM4KPSQ1
.

permcody · 2016-10-12T16:06:40Z

framework/include/vectorpostprocessors/SphericalAverage.h

  /// sample count per bin
-  VectorPostprocessorValue _count_tmp;
+  std::vector<unsigned int> _counts;


Note to reviewer: I think we should avoid creating VectoPostprocessor instances in our objects. We may eventually want to make the constructor private like we have for Material and Variable containers to help prevent users from missing the reference in normal declarations. This particular vector is not a declared vector so I switched it to C++ vector.

After this is merged we can go through the existing samplers and remove temporary vectors or other workarounds to not having TLS. refs idaholab#7809

This reverts commit 513c5df.

Also add override keywords

permcody · 2016-10-12T22:40:09Z

@friedmud - care to review this?

friedmud · 2016-10-12T22:43:32Z

Looking

friedmud

I'm marking this as Request changes... but really it's ok the way it is.

I made a few suggestions... but it's mergeable as is.

Overall this is a really great leap forward in code quality in this area. Thanks for taking the time to do it really well!

friedmud · 2016-10-12T22:45:49Z

framework/include/vectorpostprocessors/VectorPostprocessorData.h

@@ -31,6 +31,12 @@ class VectorPostprocessorData : public Restartable
   */
  VectorPostprocessorData(FEProblem & fe_problem);

+  struct VPPVectors


Maybe name it VectorPostprocessorState or something like that?

friedmud · 2016-10-12T22:50:24Z

framework/src/outputs/TableOutput.C


-    FormattedTable & table = _vector_postprocessor_tables[vpp_name];
+    auto table_it = _vector_postprocessor_tables.lower_bound(vpp_name);


This set of 3 lines could use a bit of documentation. Took me a minute to figure out that you're just looking for or adding the table.

I mean: is this really worth removing the "accidental" insertion side effect? The old code was so simple...

That's a fair criticism. I'm very, very tainted on RHS use of std::map::operator[]. I would say that I track down a bug about every 4-6 weeks that's related to careless use of that operator. I'd almost rather eradicate it's use anywhere in MOOSE but the syntax is very convenient which makes that hard to do.

This particular case is actually one of the few cases where RHS brackets are OK because we want to always create tables so I'm torn. In general, this is the syntax I'd like to move to because for the normal access case, you either add a straight up error when the item isn't found or often just an assertion when you are "sure" the item is there (yeah right...)

Here's another crazy idea... We use maps a lot in MOOSE. Maybe it's time to think about replacing our use of maps with a better container. We could start with just aliasing our container to std::map but quickly move to hiding implementation details such as the code you see there behind familiar operators like brackets. I've been using this lower_bound code lately because it's the best way to check for an item and insert if if necessary because you can use hint. Using find doesn't work because you always end up with a iterator to the end of the map meaning you have to do two tree searches to handle the insert case. Is it premature optimization? Maybe, but maybe not. I'm starting to put these in as I fix accidental insertion.

friedmud · 2016-10-12T22:50:56Z

framework/src/outputs/TableOutput.C


-      for (unsigned int i=0; i<vector.size(); i++)
+      for (auto i = beginIndex(vector); i < vector.size(); ++i)


I like this!

I do but I think we are in the minority. No other MOOSE devs are using this syntax, not even @dschwen. I wonder if we can do better still.

I used it in 513c5df after you made a comment to that effect :-D. It might just take a bit to catch on. I suggest we start converting loops (if we decide to stick with it). Seeing it in action in existing code may improve adoption.

This looks to me should be a method in vector, vector.begin_index(). This beginIndex looks ugly although correct.

Yeah, sadly there is no method for creating an enumeration. There are other options and they are all uglier.

auto i = decltype(foo)(0)
or if you don't have a const type
decltype(foo) i = 0

friedmud · 2016-10-12T22:51:24Z

framework/src/outputs/TableOutput.C


    table.clear();
    table.outputTimeColumn(false);

    for (const auto & vec_it : vectors)
    {
-      VectorPostprocessorValue vector = *(vec_it.second);


Yikes! Was this doing a copy before?!?

friedmud · 2016-10-12T22:59:14Z

framework/src/vectorpostprocessors/PointSamplerBase.C

+  _found_points.assign(_points.size(), false);
+
+  _point_values.resize(_points.size());
+  std::for_each(_point_values.begin(), _point_values.end(),


friedmud · 2016-10-12T23:10:52Z

framework/src/vectorpostprocessors/SamplerBase.C

+#ifndef NDEBUG
+  for (const auto vec_ptr : vec_ptrs)
+    if (vec_ptr->size() != vector_length)
+      mooseError("Vector length mismatch");


Yeah - it would nice to be able to actually handle this... like only sort the values that exist or something. But it's an edge case that can't happen with any of our existing objects that use this interface right now.

That'll take a different data structure because we don't have an association of which items go with which ids if the lengths are different. That's a problem for a different day.

friedmud · 2016-10-12T23:11:17Z

framework/src/vectorpostprocessors/SamplerBase.C

-    _z[i] = _z_tmp[index];
-    _id[i] = _id_tmp[index];
+    // Swap vector storage with sorted vector
+    vec_ptrs[i]->swap(tmp_vector);


friedmud · 2016-10-12T23:13:21Z

framework/src/vectorpostprocessors/VectorPostprocessorData.C

+VectorPostprocessorData::getVectorPostprocessorHelper(const VectorPostprocessorName & vpp_name, const std::string & vector_name, bool get_current)
+{
+  // Intentional use of RHS brackets on a std::map to do a multilevel retrieve or insert
+  auto & vec_struct = _values[vpp_name][vector_name];


You do it here and it's ok ;-)

Doing a double layer insert is really, really ugly with lower_bound. It's actually more than 2*3. I tried it and thought it was atrocious.

friedmud · 2016-10-12T23:19:29Z

framework/src/vectorpostprocessors/VectorPostprocessorData.C

-
-  if (pp_val == NULL)
-    pp_val = &declareRestartableDataWithObjectName<VectorPostprocessorValue>(vpp_name + "_" + vector_name, "values_old");
+  _requested_items.emplace(vpp_name + "::" + vector_name);


This means that you will create a dependency on this VPP... even though it's only a dependency on the old value of this VPP.

That can lead to erroneous cyclic dependencies (like the kind we've had to clear up elsewhere).

I think that for now it's ok to just not note the dependency on the old value. It should always be available...

Nice catch, I'll fix this

oh wait... This might not be bad depending on how it's used. We actually need three different array, or at least one additional flag: these two for creating the sanity checking (making sure that supplied and requested are on parity), but we also need a flag or something to indicate whether or not a dependency should be created for a given pair. Since this implementation isn't finished, I'll leave it for now.

friedmud · 2016-10-12T23:21:32Z

framework/src/vectorpostprocessors/VectorPostprocessorData.C

+  if (!vec_struct.current)
+  {
+    mooseAssert(!vec_struct.old, "Uninitialized pointers in VectorPostprocessor Data");
+    vec_struct.current = &declareRestartableDataWithObjectName<VectorPostprocessorValue>(vpp_name + "_" + vector_name, "values");


Yuck - does this have to be this way for "lazy" binding? It seems like the restartable data should be declared when the vector is declared.

(Note: I realize that the old code was doing the same)

I'm sure we can do better. This ...Data class is isolated from users so I'm not worried about fixing this right now. These things should be on the radar later for refactoring opportunities.

permcody · 2016-10-13T16:02:30Z

Comments have been addressed.

permcody · 2016-10-18T21:34:11Z

This PR should be ready to merge, any takers?

permcody · 2016-10-20T14:56:11Z

@andrsd - Would you mind reviewing this PR and merging if appropriate? It's been sitting here for awhile.

friedmud · 2016-11-11T22:44:41Z

@permcody This PR breaks my application...

Like I said: I need to be able to declare vectors after the solve. I thought this wasn't going to mess with that?

Reverting this merge commit fixes my problem...

How do you want to proceed? This is required for my application and my immediate deliverables. Can you see an easy path forward?

permcody · 2016-11-12T00:41:38Z

I guess we need to get your App fixed and on our our testing list! I'm sure
we can accomodate the design of this PR with your needs. Are you just
getting an error because something isn't already declared?

On Fri, Nov 11, 2016 at 3:44 PM Derek Gaston notifications@github.com
wrote:

@permcody https://github.com/permcody This PR breaks my application...

Like I said: I need to be able to declare vectors after the solve. I
thought this wasn't going to mess with that?

Reverting this merge commit fixes my problem...

How do you want to proceed? This is required for my application and my
immediate deliverables. Can you see an easy path forward?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7819 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AC5XIEb2EspJ0VD8oQchO5Pi3PokJg-Sks5q9O_agaJpZM4KPSQ1
.

friedmud · 2016-11-12T00:46:11Z

I get a segfault in opt. In dbg I get an error about a key missing in
VectorPostprocessorData.

On Fri, Nov 11, 2016 at 7:41 PM Cody Permann notifications@github.com
wrote:

I guess we need to get your App fixed and on our our testing list! I'm sure
we can accomodate the design of this PR with your needs. Are you just
getting an error because something isn't already declared?

On Fri, Nov 11, 2016 at 3:44 PM Derek Gaston notifications@github.com
wrote:

@permcody https://github.com/permcody This PR breaks my application...

Like I said: I need to be able to declare vectors after the solve. I
thought this wasn't going to mess with that?

Reverting this merge commit fixes my problem...

How do you want to proceed? This is required for my application and my
immediate deliverables. Can you see an easy path forward?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7819 (comment),
or mute
the thread
<
https://github.com/notifications/unsubscribe-auth/AC5XIEb2EspJ0VD8oQchO5Pi3PokJg-Sks5q9O_agaJpZM4KPSQ1

.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7819 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AA1JMc1PN5WFW2XHwlkQZeEqRK2z0_zBks5q9QtCgaJpZM4KPSQ1
.

permcody · 2016-11-12T01:13:06Z

Is that without threads? I'm surprised that anything changed for the
non-threaded case.

On Fri, Nov 11, 2016 at 5:46 PM Derek Gaston notifications@github.com
wrote:

I get a segfault in opt. In dbg I get an error about a key missing in
VectorPostprocessorData.

On Fri, Nov 11, 2016 at 7:41 PM Cody Permann notifications@github.com
wrote:

I guess we need to get your App fixed and on our our testing list! I'm
sure
we can accomodate the design of this PR with your needs. Are you just
getting an error because something isn't already declared?

On Fri, Nov 11, 2016 at 3:44 PM Derek Gaston notifications@github.com
wrote:

@permcody https://github.com/permcody This PR breaks my
application...

Like I said: I need to be able to declare vectors after the solve. I
thought this wasn't going to mess with that?

Reverting this merge commit fixes my problem...

How do you want to proceed? This is required for my application and my
immediate deliverables. Can you see an easy path forward?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7819 (comment),
or mute
the thread
<

https://github.com/notifications/unsubscribe-auth/AC5XIEb2EspJ0VD8oQchO5Pi3PokJg-Sks5q9O_agaJpZM4KPSQ1

.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7819 (comment),
or mute
the thread
<
https://github.com/notifications/unsubscribe-auth/AA1JMc1PN5WFW2XHwlkQZeEqRK2z0_zBks5q9QtCgaJpZM4KPSQ1

.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7819 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AC5XIKyjU0t1lQgmUtBTxfOvW0b5xBr9ks5q9QxUgaJpZM4KPSQ1
.

friedmud · 2016-11-12T01:14:27Z

Yep - without threads.

On Fri, Nov 11, 2016 at 8:13 PM Cody Permann notifications@github.com
wrote:

Is that without threads? I'm surprised that anything changed for the
non-threaded case.

On Fri, Nov 11, 2016 at 5:46 PM Derek Gaston notifications@github.com
wrote:

I get a segfault in opt. In dbg I get an error about a key missing in
VectorPostprocessorData.

On Fri, Nov 11, 2016 at 7:41 PM Cody Permann notifications@github.com
wrote:

I guess we need to get your App fixed and on our our testing list! I'm
sure
we can accomodate the design of this PR with your needs. Are you just
getting an error because something isn't already declared?

On Fri, Nov 11, 2016 at 3:44 PM Derek Gaston <notifications@github.com

wrote:

@permcody https://github.com/permcody This PR breaks my
application...

Like I said: I need to be able to declare vectors after the solve. I
thought this wasn't going to mess with that?

Reverting this merge commit fixes my problem...

How do you want to proceed? This is required for my application and
my
immediate deliverables. Can you see an easy path forward?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7819 (comment)
,
or mute
the thread
<

https://github.com/notifications/unsubscribe-auth/AC5XIEb2EspJ0VD8oQchO5Pi3PokJg-Sks5q9O_agaJpZM4KPSQ1

.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7819 (comment),
or mute
the thread
<

https://github.com/notifications/unsubscribe-auth/AA1JMc1PN5WFW2XHwlkQZeEqRK2z0_zBks5q9QtCgaJpZM4KPSQ1

.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7819 (comment),
or mute
the thread
<
https://github.com/notifications/unsubscribe-auth/AC5XIKyjU0t1lQgmUtBTxfOvW0b5xBr9ks5q9QxUgaJpZM4KPSQ1

.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7819 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AA1JMSn454yAd-wCbYJ22XWHQN7Oei0gks5q9RKjgaJpZM4KPSQ1
.

friedmud · 2016-11-12T01:16:29Z

If you want to run it:

https://github.com/friedmud/mocodile

The test is:

deterministic/parameter_study

friedmud · 2016-11-12T21:33:28Z

Here's what I get in opt... I get a segfault here:

* thread #1: tid = 0x30abfa, 0x0000000100672a00 libmoose-opt.0.dylib`TableOutput::outputVectorPostprocessors() + 592, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x38)
  * frame #0: 0x0000000100672a00 libmoose-opt.0.dylib`TableOutput::outputVectorPostprocessors() + 592
    frame #1: 0x0000000100645d5e libmoose-opt.0.dylib`CSV::outputVectorPostprocessors() + 14
    frame #2: 0x000000010062eee9 libmoose-opt.0.dylib`AdvancedOutput<FileOutput>::output(ExecFlagType const&) + 1257
    frame #3: 0x0000000100645daf libmoose-opt.0.dylib`CSV::output(ExecFlagType const&) + 63
    frame #4: 0x000000010066b12c libmoose-opt.0.dylib`OutputWarehouse::outputStep(ExecFlagType) + 76
    frame #5: 0x000000010045bceb libmoose-opt.0.dylib`MooseApp::executeExecutioner() + 75
    frame #6: 0x000000010045cab6 libmoose-opt.0.dylib`MooseApp::run() + 118
    frame #7: 0x0000000100001522 mocodile-opt`main + 114
    frame #8: 0x00007fff970dc5ad libdyld.dylib`start + 1
    frame #9: 0x00007fff970dc5ad libdyld.dylib`start + 1

friedmud · 2016-11-12T21:39:00Z

In dbg I get this:

Assertion `vec_pair != _values.end()' failed
No vectors found for vpp_name: ray_tracing
at /Users/gastdr/projects/moose/framework/src/vectorpostprocessors/VectorPostprocessorData.C, line 74
Stack frames: 15
0: 0   libmesh_dbg.0.dylib                 0x000000010454a08f libMesh::print_trace(std::__1::basic_ostream<char, std::__1::char_traits<char> >&) + 2287
1: 1   libmoose-dbg.0.dylib                0x00000001035fcab9 VectorPostprocessorData::vectors(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) const + 2457
2: 2   libmoose-dbg.0.dylib                0x00000001026653df FEProblem::getVectorPostprocessorVectors(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) + 47
3: 3   libmoose-dbg.0.dylib                0x0000000103126cc5 TableOutput::outputVectorPostprocessors() + 581
4: 4   libmoose-dbg.0.dylib                0x00000001030a466f CSV::outputVectorPostprocessors() + 31
5: 5   libmoose-dbg.0.dylib                0x0000000103068204 AdvancedOutput<FileOutput>::output(ExecFlagType const&) + 2052
6: 6   libmoose-dbg.0.dylib                0x00000001030a46dd CSV::output(ExecFlagType const&) + 93
7: 7   libmoose-dbg.0.dylib                0x0000000103017cc6 AdvancedOutput<FileOutput>::outputStep(ExecFlagType const&) + 182
8: 8   libmoose-dbg.0.dylib                0x000000010310a4d5 OutputWarehouse::outputStep(ExecFlagType) + 453
9: 9   libmoose-dbg.0.dylib                0x0000000102670075 FEProblem::outputStep(ExecFlagType) + 101
10: 10  libmoose-dbg.0.dylib                0x0000000102d110dd Steady::init() + 157
11: 11  libmoose-dbg.0.dylib                0x0000000102a94bd4 MooseApp::executeExecutioner() + 180
12: 12  libmoose-dbg.0.dylib                0x0000000102a9733c MooseApp::run() + 140
13: 13  mocodile-dbg                        0x0000000101c8f3df main + 303
14: 14  libdyld.dylib                       0x00007fff970dc5ad start + 1

friedmud · 2016-11-12T21:40:18Z

That last one is spot on... there are no vectors for that VPP... because it hasn't declared any yet (this is initial output)

friedmud · 2016-11-12T21:44:45Z

The basic issue is that VectorPostprocessor::vectors() and FEProbem::getVectorPostprocessorVectors() both return references... but if there are no vectors there is nothing to return.

friedmud · 2016-11-12T21:50:23Z

Trying to fix it - stay tuned

idaholab#7819

aeslaughter reviewed Oct 6, 2016

View reviewed changes

friedmud requested changes Oct 6, 2016

View reviewed changes

permcody force-pushed the vpp_tls branch from 6343a1f to 3a9bb1c Compare October 7, 2016 21:21

permcody force-pushed the vpp_tls branch from 3a9bb1c to 5e382ed Compare October 7, 2016 21:24

permcody mentioned this pull request Oct 7, 2016

Add spherical average vector postprocessor #7828

Merged

permcody added the PR: Review Requested label Oct 7, 2016

permcody force-pushed the vpp_tls branch from 5e382ed to 41f1210 Compare October 7, 2016 21:38

dschwen reviewed Oct 11, 2016

View reviewed changes

permcody force-pushed the vpp_tls branch 2 times, most recently from 421472f to 56cb41b Compare October 12, 2016 16:03

permcody commented Oct 12, 2016

View reviewed changes

permcody force-pushed the vpp_tls branch from 56cb41b to 34dcfc1 Compare October 12, 2016 16:38

permcody added 7 commits October 12, 2016 12:19

TLS for VectorPostprocessors

59fd021

After this is merged we can go through the existing samplers and remove temporary vectors or other workarounds to not having TLS. refs idaholab#7809

Revert "Fix thread fail (idaholab#7801)"

2705db3

This reverts commit 513c5df.

Fix sign comparison warning

2569c4d

Also add override keywords

Rewrite SamplerBase to avoid temporaries

119e35f

Minor refactor of PointerSamplerBase

2948396

Fix broken test detected by assert

1de886a

Update SphericalAverage VPP to avoid temporaries

7be7053

permcody force-pushed the vpp_tls branch from 34dcfc1 to 7be7053 Compare October 12, 2016 18:19

friedmud requested changes Oct 12, 2016

View reviewed changes

Address comments

adcc4e9

permcody force-pushed the vpp_tls branch from 54a20dd to adcc4e9 Compare October 13, 2016 16:17

permcody added PR: Ready To Merge and removed PR: Review Requested labels Oct 13, 2016

permcody assigned andrsd Oct 20, 2016

andrsd merged commit 0fc2aee into idaholab:devel Oct 20, 2016

friedmud mentioned this pull request Nov 12, 2016

Deal with the case where a VPP doesn't declare vectors #8060

Merged

friedmud added a commit to friedmud/moose that referenced this pull request Nov 12, 2016

Add test for a VPP that declares a vector fairly late refs idaholab#7809

28f2371

idaholab#7819

permcody mentioned this pull request Jan 9, 2017

VectorPostprocessor threads design #7809

Closed

jwpeterson removed the PR: Ready To Merge label Apr 18, 2017

permcody deleted the vpp_tls branch September 20, 2017 23:48


		FormattedTable & table = _vector_postprocessor_tables[vpp_name];
		auto table_it = _vector_postprocessor_tables.lower_bound(vpp_name);


		for (unsigned int i=0; i<vector.size(); i++)
		for (auto i = beginIndex(vector); i < vector.size(); ++i)

TLS for VectorPostprocessors #7819

TLS for VectorPostprocessors #7819

Conversation

permcody commented Oct 5, 2016

Choose a reason for hiding this comment

permcody Oct 6, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

friedmud commented Oct 6, 2016

permcody commented Oct 6, 2016

permcody commented Oct 7, 2016

dschwen commented Oct 7, 2016

permcody commented Oct 7, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

friedmud commented Oct 11, 2016

permcody commented Oct 11, 2016

friedmud commented Oct 12, 2016

Choose a reason for hiding this comment

permcody commented Oct 12, 2016

friedmud commented Oct 12, 2016

friedmud left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

permcody commented Oct 13, 2016

permcody commented Oct 18, 2016

permcody commented Oct 20, 2016

friedmud commented Nov 11, 2016

permcody commented Nov 12, 2016

friedmud commented Nov 12, 2016

permcody commented Nov 12, 2016

friedmud commented Nov 12, 2016

friedmud commented Nov 12, 2016

friedmud commented Nov 12, 2016

friedmud commented Nov 12, 2016

friedmud commented Nov 12, 2016

friedmud commented Nov 12, 2016

friedmud commented Nov 12, 2016

permcody Oct 6, 2016 •

edited