number of tidal cycles in one morphological year (N) > 1

[IgnaceP]

Hi,

I am playing around with increasing the number of tidal cycles in one morphological year (N variable in the run.py script in the examples). However, this results in an error in line 199 when i (for i in range(N) loop) is 1:

# Export next initial condition.
tel.export('./tmp.slf', step = -1)

with the error being:

Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/IPython/core/interactiveshell.py", line 3550, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-10-be63fb26f594>", line 199, in <module>
    tel.export('./tmp.slf', step = -1)
  File "/Users/ignace/Custom_Libraries/demeter/telemac.py", line 359, in export
    data[vid, :] = self.cov[step, :]
IndexError: index -1 is out of bounds for axis 0 with size 0

After the first iteration (i = 0), a new time step is added to tel instance for all variables, except for the 'coverage' variable, as this is done outside the for i in range(N) loop, at the end of the cellular automaton (line 303). However, as the the last time step is removed at the end of the for i in range(N) loop (line 232), this results in an empty tel.cov array and tel cannot be exported. I tried to fix this by appending a coverage state every iteration (for i in range(N)) loop, except the last one (the cellular automaton will take care of this).

I am curious what your thoughts are one this!

[ogourgue]

Hi Ignace,

Good catch, thanks for pointing it out!

I think I never really tested simulations with N > 1, my (very) bad. Your suggestion seems good to me. I managed to have the saltmarsh exemple (v8p2r1) running with adding these lines:

        # Apply sea level rise to the next initial condition.
        tel.s[-1, :] += SLRR / N

        ### START NEW CODE ###
        # Coverage not updated between two steps of intra-annual loop.
        # If not final step, add dummy coverage variable before removing
        # previous time step.
        if i < N: tel.append_variable(tel.cov[-1, :].reshape(1, -1), 'coverage')
        ### END NEW CODE ###

        # Remove previous time step.
        tel.remove_time_step()

If you can confirm this solves the issue, I will update the examples, and maybe add one that explicitly test N > 1 to make sure we keep it working.

Another possibility might be to update the remove_time_step function of the Telemac class, with the possibility to avoid removing time step to some variables.

What do you think?

Either way, this would require an update of the code (or the examples), so I will move this to an issue.

Thanks again!

[IgnaceP]

That was also how I solved it, altough if i < N-1:

(you want to append it every iteration, except the last so if N = 4, you want it at i = 0, i = 1 and i = 2)
or am I missing something?

[ogourgue]

Of course, you're right. That's why I had this weird tel.cov[-1, :].reshape(1, -1)

The solution is:

        # Apply sea level rise to the next initial condition.
        tel.s[-1, :] += SLRR / N

        ### START NEW CODE ###
        # Coverage not updated between two steps of intra-annual loop.
        # If not final step, add dummy coverage variable before removing
        # previous time step.
        if i < N - 1: tel.append_variable(tel.cov, 'coverage')
        ### END NEW CODE ###

        # Remove previous time step.
        tel.remove_time_step()

If you think that's good enough, I'll update the examples.

[IgnaceP]

I tested it too, and it works!

[ogourgue]

Examples updated. New ones added with N = 2. Thanks @IgnaceP !