Code extension to compute LDs of discrete systems.

[vkrajnak]

There is a lot of stuff in Discrete_Lagrangian_Descriptors_NOTES, how is it meant to be read? The part with writing modules into a .py file is an unusual choice.

[vkrajnak]

Standard map in Discrete_Lagrangian_Descriptors_NOTES: if f is the map, then why is there something added to the result outside of the function?

        points_next = f(points_initial, PARAMETERS=[1.2])
        points_next = points_next - np.floor(points_next + 1/2)

The line points_next = points_next - np.floor(points_next + 1/2) is only needed for visualization of Poincare plots. Not for LD computation, since you're meant to take trajectory "lengths".

[vkrajnak]

Is there any particular advantage of using itemgetter as in N_points_slice_axes = list( map(itemgetter(-1), grid_parameters)) over N_points_slice_axes = grid_parameters[:,-1] which is moreover a numpy array?

Moreover N_points_slice_axes should be renamed to something that reflects the contained information about shape of an array.

Edit: this seems to concern the existing version of base.py too

[vkrajnak]

Hénon's name is misspelled on several occasions and if we use accents, Poincaré contains one as well.

[broncio123]

There is a lot of stuff in Discrete_Lagrangian_Descriptors_NOTES, how is it meant to be read? The part with writing modules into a .py file is an unusual choice.

This NB is meant to be a collection of scratch notes in the same spirit as Lagrangian_Descriptors_Python_NOTES. It contains tests I had to run to verify myself the correctness of the definition of the maps. The section Modules is the only one concerning the code for implementation.

[broncio123]

Is there any particular advantage of using itemgetter as in N_points_slice_axes = list( map(itemgetter(-1), grid_parameters)) over N_points_slice_axes = grid_parameters[:,-1] which is moreover a numpy array?

Moreover N_points_slice_axes should be renamed to something that reflects the contained information about shape of an array.

Edit: this seems to concern the existing version of base.py too

Yep, there's a big difference. If grid_parameters is defined as a np.array then grid_parameters[:,-1] works fine. BUT, if the user accidentally decides to input grid_parameters as a list (which is how we exemplify in NB Examples), then, the above syntax doesn't work. By contrast, list( map(itemgetter(-1), grid_parameters)) works for both, np.array and list. Slightly more convoluted syntax, but more robust to object-type error.

[broncio123]

Hénon's name is misspelled on several occasions and if we use accents, Poincaré contains one as well.

Got ya!

[vkrajnak]

Yep, there's a big difference. If grid_parameters is defined as a np.array then grid_parameters[:,-1] works fine. BUT, if the user accidentally decides to input grid_parameters as a list (which is how we exemplify in NB Examples), then, the above syntax doesn't work. By contrast, list( map(itemgetter(-1), grid_parameters)) works for both, np.array and list. Slightly more convoluted syntax, but more robust to object-type error.

If it's just about np.arrays and lists, then something like np.array(grid_parameters)[:,-1] should work and be readable. Could it ever happen that grid_parameters is a dictionary, as you implemented for the 2DoF systems?

[broncio123]

Yep, there's a big difference. If grid_parameters is defined as a np.array then grid_parameters[:,-1] works fine. BUT, if the user accidentally decides to input grid_parameters as a list (which is how we exemplify in NB Examples), then, the above syntax doesn't work. By contrast, list( map(itemgetter(-1), grid_parameters)) works for both, np.array and list. Slightly more convoluted syntax, but more robust to object-type error.

If it's just about np.arrays and lists, then something like np.array(grid_parameters)[:,-1] should work and be readable. Could it ever happen that grid_parameters is a dictionary, as you implemented for the 2DoF systems?

If np.array(grid_parameters)[:,-1] done, then output array entries are floats and not ints as defined in grid_parameters. reshape can only take ints as inputs. Former approach respects data types.

[broncio123]

Standard map in Discrete_Lagrangian_Descriptors_NOTES: if f is the map, then why is there something added to the result outside of the function?

        points_next = f(points_initial, PARAMETERS=[1.2])
        points_next = points_next - np.floor(points_next + 1/2)

The line points_next = points_next - np.floor(points_next + 1/2) is only needed for visualization of Poincare plots. Not for LD computation, since you're meant to take trajectory "lengths".

Yep, that's right. These lines are only used to generate P maps. In the computation of LDs, this is never done.

[broncio123]

Is there any particular advantage of using itemgetter as in N_points_slice_axes = list( map(itemgetter(-1), grid_parameters)) over N_points_slice_axes = grid_parameters[:,-1] which is moreover a numpy array? Moreover N_points_slice_axes should be renamed to something that reflects the contained information about shape of an array. Edit: this seems to concern the existing version of base.py too

Yep, there's a big difference. If grid_parameters is defined as a np.array then grid_parameters[:,-1] works fine. BUT, if the user accidentally decides to input grid_parameters as a list (which is how we exemplify in NB Examples), then, the above syntax doesn't work. By contrast, list( map(itemgetter(-1), grid_parameters)) works for both, np.array and list. Slightly more convoluted syntax, but more robust to object-type error.

Consider list comprehension instead 👍🏾

[broncio123]

Yep, there's a big difference. If grid_parameters is defined as a np.array then grid_parameters[:,-1] works fine. BUT, if the user accidentally decides to input grid_parameters as a list (which is how we exemplify in NB Examples), then, the above syntax doesn't work. By contrast, list( map(itemgetter(-1), grid_parameters)) works for both, np.array and list. Slightly more convoluted syntax, but more robust to object-type error.

If it's just about np.arrays and lists, then something like np.array(grid_parameters)[:,-1] should work and be readable. Could it ever happen that grid_parameters is a dictionary, as you implemented for the 2DoF systems?

Maps will remain 2d only and hence grid parameters won’t be a dictionary.

[broncio123]

Edit docstrings of all functions in all modules. This is something we totally overlooked! @vkrajnak

[vkrajnak]

Hénon's name is misspelled in the examples notebook

[vkrajnak]

On quite many occasions, the code mentions 'discrete maps'. This is confusing as none of the maps is discrete. We use these (continuous) maps to define a discrete dynamical system, but the maps are not discrete.

[broncio123]

On quite many occasions, the code mentions 'discrete maps'. This is confusing as none of the maps is discrete. We use these (continuous) maps to define a discrete dynamical system, but the maps are not discrete.

I agree. I abused terminology for variable naming in the Examples NB, as a way to differentiate between a map (as a variable name) and the built-in function map. So, discrete_ can be just taken as a prefix.

But, if you have any other suggestion for variable naming of maps, let me know.

[vkrajnak]

As discussed, periodicity gets fixed after correcting y0 values for PBC as

y0_pbc = y0 - np.floor(y0 + 1/2) (Jame Meiss' mod function definition, periodic cell centred at origin) and then feeding these into lagrangian_descriptor as

LD_values = LD_values + lagrangian_descriptor(y0_pbc, dy, p_value) for p-value = 0 (Action-based LD)

It's probably just me again, but where in the code was this line added? I can't see it in base_discrete.py on this branch

[broncio123]

Graphic representation of how given a periodic box (PB) which is off-centre in relation to the domain of initial conditions (ICs) will see points in the domain (y0) and their iterations (f(y0)). This shows how the origin of the PB has an effect on coordinates corrected by PBCs.

Note that if the domain of ICs and the PB coincide, then there is no change in the values of y0 and f(y0). What do you think? @vkrajnak @VikJGG