enable volatile params

[Yurlungur]

PR Summary

This is motivated by @brryan 's comment in issue #621 . I modify Params to enable the user to tag a param as "volatile." The effect of this is that the user can optionally extract the non-const pointer from the Params object with GetVolatile in the Params object or VolatileParam in the StateDescriptor object. If the user attempts to extract a non-volatile param with GetVolatile, a runtime error is raised.

I thought about returning a non-const reference, but the pointer felt safer and more explicit, without being any more cumbersome really. On the other hand, returning a pointer with GetVolatile means that the two Get methods return different types, which is a little weird. So I'd be open to returning a non-const reference instead, if people would like that better.

PR Checklist

• Code passes cpplint
• New features are documented.
• Adds a test for any bugs fixed. Adds tests for new features.
• Code is formatted
• Changes are summarized in CHANGELOG.md
• CI has been triggered on Darwin for performance regression tests.
• (@lanl.gov employees) Update copyright on changed files

[Yurlungur]

Pinging @brryan @pgrete @jdolence @nmsriram @forrestglines as people with stakes in this machinery and/or who wrote the original version

[dholladay00]

small nomenclature suggestion: find-replace volatile with mutable.

[Yurlungur]

small nomenclature suggestion: find-replace volatile with mutable.

This is a good idea.

[Yurlungur]

small nomenclature suggestion: find-replace volatile with mutable.

done.

[pgrete]

👍 I like this approach to be more specific.

The one additional change I'd like to see before approving is an update to the Update function.
From a consistency point of view, I think that if we differentiate between mutable and non-mutable parameter, then Update should also check if the parameter is mutable and otherwise throw an error.

[pgrete]

Out of curiosity, is there a reason to use emplace in one case and the direct assignment in the other case?
Also, I wonder, if it wouldn't be cleaner the only use one map (e.g., with a struct) instead of three, but that's probably a separate discussion.

[Yurlungur]

I used assignment because it was easier. I don't know why the original was emplace. I can copy that if it's preferred.

[Yurlungur]

@pgrete take a look now. I added a check for mutable in Params::Update and renamed make_mutable to is_mutable.

[Yurlungur]

Pinging @gshipman to review and @pgrete to approve as discussed on the parthenon call.

[pgrete]

Looks good to go!

[codecov]

Codecov Report

Merging #623 (ae694ee) into develop (a671af0) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #623      +/-   ##
===========================================
+ Coverage    54.25%   54.28%   +0.02%     
===========================================
  Files          125      125              
  Lines        13699    13701       +2     
===========================================
+ Hits          7433     7438       +5     
+ Misses        6266     6263       -3     

Impacted Files	Coverage Δ
src/interface/params.hpp	90.90% <100.00%> (+7.57%)	⬆️
src/interface/state_descriptor.hpp	95.83% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a671af0...ae694ee. Read the comment docs.