Add SmartField usage to some legacy algorithms #1236
Conversation
| } | ||
| } | ||
| thermalCond_->modify_on_host(); |
There was a problem hiding this comment.
It's nice to be able to remove these modify and sync calls.
| const std::string& name, | ||
| const stk::mesh::PartVector& parts, | ||
| const int numStates = 0, | ||
| const int numComponents = 0, | ||
| const void* init_val = nullptr); | ||
| const void* init_val = nullptr) const; |
There was a problem hiding this comment.
I'm curious how field registry can be a compile-time definition, if it depends on a part-vector which would have run-time contents.
There was a problem hiding this comment.
I was thinking the type look up could be moved to compile time so that when fields are accessed we wouldn't have to include the type. Registry would still have to be runtime. However, looking at the dependencies I'm not sure I want to do that anymore. We'd have to expose more of the field information across the code base instead of isolating it to a single *.C file. This could wreak havoc on the compile times. After we convert to the simple fields the type information becomes way less cumbersome for accessing fields. So I think we should keep it as is.
|
@psakievich I think that's a great idea! Both old-style and new-style We will still have to convert a few parts of nalu-wind beyond just the I'll get to work right away on this. I'll base my changes on current master nalu-wind, with this |
|
@djglaze Sounds great. I like that we have a compile time check for checking when we are close to converting all the fields. I think it will go relatively quickly and should be a net code reduction. Algorithms are straightforward to convert. I need to look at the machinery behind the kernels next. Once I get that squared away we should be able to do a group conversion swarm on the entire code base. |
|
@psakievich Upon closer inspection, I'm going to temper my enthusiasm for this approach. It would be a much bigger change than I was thinking, and it would make your The issue is all of the So, I'm now thinking that it would be best to save the simple_fields stuff until after a complete |
|
@djglaze okay this sounds like what I ran up against when I tried to merge minimal changes from your PR into this one. I would say either way we have to extensively touch the code base twice. If you know what the issue was with the intel build it might be better to revive #1233 and do it first? We can revive it and test it on multiple machines before merging. |
|
@psakievich I do know what the problem was with the original #1233 commit. I'd be perfectly happy with resurrecting that, if you like. From my testing on both gcc and Intel, I don't believe there are any other problems in there (other than the one that @jrood-nrel pointed out). It's a big commit, but the work is already done and it shouldn't interfere with any of your upcoming |
|
@djglaze that is what I am thinking too. I think if we do the |
|
@psakievich Cool. I'll get that going, then. |
@djglaze would it be possible to include the updates from #1233 for just the
SmartField/FieldManageraccessors? I would like to usefieldManager_.get_legacy_smart_field<double>("temperature")for the conversion process. My thought is if we update this interface then we can just make the update to simple fields as we convert to smart fields. I started playing with it but I got some type mismatch issues.I'm also considering if I can make the
FeildRegistryaconstexprtype using the compile time const map from Jason Turner's training. Maybe we can just move the field type analysis to compile time and remove some templates if we revisit that design. @overfelt and @alanw0 you guys might be interested in that as well.