Skip to content

Conversation

@MarcoCerino23
Copy link
Contributor

I've added the FullDelayESN model.

This model combines both delay mechanisms independently, allowing for distinct configurations:

  • Input Delays: Controlled by num_input_delays and input_stride (operates before the reservoir).
  • State Delays: Controlled by num_state_delays and state_stride (operates after the reservoir).

I've implemented the struct and added comprehensive tests (all passing locally)

Checklist

@MartinuzziFrancesco
Copy link
Collaborator

This is essentially ready to go! I've just added a couple of minor comments. Also, I think that naming wise we could change your FullDelayESN to be named as DelayESN, and change the current DelayESN to be called StateDelayESN. Does it makes sense?

- Renamed DelayESN -> StateDelayESN
- Renamed FullDelayESN -> DelayESN (now the main model)
- Merged all delay models (Input, State, Delay) into src/models/esn_delay.jl
- Merged all tests into test/models/test_esn_delay.jl
- Cleaned up exports and includes in ReservoirComputing.jl
@MarcoCerino23
Copy link
Contributor Author

Absolutely, I totally agree! That naming convention makes the hierarchy much clearer and more intuitive.

Here is a summary of the changes in the latest commit:

  1. Renaming:
    • FullDelayESNDelayESN (since it covers both delay types).
    • The previous DelayESNStateDelayESN.
  2. File Consolidation: As suggested, I moved all three delay models (InputDelayESN, StateDelayESN, DelayESN) into a single file: src/models/esn_delay.jl. I did the same for the tests in test/models/test_esn_delay.jl and removed the obsolete files.
  3. Refactoring: I updated the new DelayESN struct so that the state delay layer is handled inside states_modifiers.

I believe everything is clean and ready now!

@MartinuzziFrancesco
Copy link
Collaborator

awesome, we merge as soon as tests pass!

@MartinuzziFrancesco MartinuzziFrancesco merged commit 5168e14 into SciML:master Jan 26, 2026
18 checks passed
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.

2 participants