Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added a copy #72

Merged
merged 2 commits into from
Nov 16, 2021
Merged

added a copy #72

merged 2 commits into from
Nov 16, 2021

Conversation

odunbar
Copy link
Collaborator

@odunbar odunbar commented Nov 15, 2021

Purpose

Resolves #71, when we load data from data storage we should always copy, as it is history and should not be overwritable

Example

I was recently stung by the old mutability vs assignment:

x=ones(10)
f() = return x

then

u1 =  f()
u1 += ones(10)
# u1 = 2*ones(10)
# x = ones(10)

and

u2 = f()
u2 = 2*ones(10)
# u2 = 2*ones(10)
# x = ones(10)

But(!)

u3 = f()
u3[:] = 2*ones(10)
# u2 = 2*ones(10)
# x = 2*ones(10)

let's not rely on people like me to remember this in e.g. the getters from storage get_u,get_u_final,get_u_prior etc.
A copy in the get_data method of the DataStorage class resolves this

x=ones(10)
g() = return copy(x)
u4 = g()
u4[:] = 2*ones(10)
# u4 = 2*ones(10)
# x = ones(10)

@jakebolewski
Copy link
Contributor

You probably want to use deepcopy otherwise LGTM

@bielim
Copy link
Contributor

bielim commented Nov 16, 2021

Looks good to me!
For some reason this instance of the problem never bit me, but with python I fell into that same trap a couple of times. And I had to learn that the trap also exists in languages like C++, where you'd think that because everything is more explicit you might be less prone to accidentally changing the values of variables you don't want to change, for example:

#include <algorithm>  
#include <vector>

#include <cassert>

using namespace std;

int main() {

  float const pi  { 3.1415f };

  // x = [1.,1.,1.]
  auto x = vector<float>(3, 1.f);

  // A C++ reference
  auto& u = x;

  // When you assign to a reference in C++ it is as if you assigned to the 
  // original value (because the reference has value semantics!), so in
  // contrast to Julia (and python), the following re-assignment will
  // actually change x as well:
  u = std::vector<float>(3, pi);

  // So x now also became pi
  assert(all_of(x.cbegin(), x.cend(), [&](auto v){ return v == pi; }));

  return 0;
}

Yay :-)

@odunbar
Copy link
Collaborator Author

odunbar commented Nov 16, 2021

bors r+

@bors
Copy link
Contributor

bors bot commented Nov 16, 2021

Build succeeded:

@bors bors bot merged commit da4b7da into main Nov 16, 2021
@bors bors bot deleted the orad/data_store branch November 16, 2021 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

I'm worried that we don't do enough copies.
4 participants