Fix POP restart from omuse provided data

[esclapez]

• POP internally has two versions of the prognostic variables: old and current. Previous implementation only exposed the current one in getter, and setter functions duplicated the data in both old and current POP arrays. New implementation uses suffixes _old and _cur to get/set old and current respectively, while a _all suffix is implemented to keep the previous behavior:

  channel.copy_attributes(["xvel_cur"])
  

  will copy ONLY the current data between two instances.

  channel.copy_attributes(["xvel_old"])
  

  will copy ONLY the old data between two instances.

  channel.copy_attributes(["xvel_all"])
  

  will copy the current data into both old and current between two instances.
• when restarting a POP instance from data provided through the Omuse mechanism described above, one need to specify:

  p.parameters.ts_option = "amuse"
  p.parameters.pressure_correction = False
  

  to ensure a smooth restart.

[esclapez]

@Sbte can you get a look at this ? It should include the changes we discussed at IMAU the other day.

[Sbte]

It seems like a lot of code duplication to me. Would it be possible to add an extra oldtime argument to all Fortran setters and getters, such that they can be implemented in Python as something like

def set_node_barotropic_vel_oldtime(self, i, j, vx, vy):
    return self.set_node_barotropic_vel(i, j, vx, vy, True)

def set_node_barotropic_vel_curtime(self, i, j, vx, vy):
    return self.set_node_barotropic_vel(i, j, vx, vy, False)

def set_node_barotropic_vel(self, i, j, vx, vy):
    return self.set_node_barotropic_vel(i, j, vx, vy, True)
    return self.set_node_barotropic_vel(i, j, vx, vy, False)

def get_node_barotropic_vel_oldtime(self, i, j):
    return self.get_node_barotropic_vel(i, j, True)

def get_node_barotropic_vel_curtime(self, i, j):
    return self.get_node_barotropic_vel(i, j, False)

def get_node_barotropic_vel(self, i, j):
    return self.get_node_barotropic_vel(i, j, False)

Not sure if this is possible, but it avoids the code duplication.

[Sbte]

Also, please squash the fixes into earlier commits. You can easily do this with git rebase -i and then use fixup.

[esclapez]

@Sbte I agree, way too much duplication but I can't seem to make a more generic version work within the Amuse interface stuff. I'd rather go with this for now so you can use it, and revisit later when I have a better grasp of how to make that work.

[Sbte]

Yeah, that's probably a good idea. I'll test it next week and then merge it if it works.

[Sbte]

Note that this way of handling the _all attributes also duplicates them in the data files, which is not optimal.

[Sbte]

Is it necessary that ts_option can only be set from the INITIALIZED state? This is not required for reinit_gradp for instance.

[Sbte]

Model time does not seem to be stored anywhere. Is that fixable in a reasonable amount of time? If not I don't think it matters.

[esclapez]

Handling of time is cumbersome in POP. If you really need to sync time, the easiest way is to set it to your desired value in the namelist input file.

[esclapez]

After commit 21f01c0, all the _all variables have been removed, reducing code duplication and extra disk space. The current time variable are accessible without extension:

channel.copy_attributes(["xvel"])

Both xvel and xvel_old have to be copied to mimic the previous behavior. For ssh the guess pressure also have to copied (on top of ssh and ssh_old):

channel.copy_attributes(["ssh_guess"])

[esclapez]

@Sbte Let me know if you think this is ready to merge or if you see something else to update.