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

Need a data structure for time series input to the NN model #4

Open
gajanan-choudhary opened this issue Oct 2, 2020 · 2 comments · May be fixed by #3
Open

Need a data structure for time series input to the NN model #4

gajanan-choudhary opened this issue Oct 2, 2020 · 2 comments · May be fixed by #3
Assignees
Labels
enhancement New feature or request

Comments

@gajanan-choudhary
Copy link
Member

I think we should create a data structure for time series input to the NN model to separate out time-stepping from input series. We must account for two things in that:

  • The input series time stamps may not coincide with the NN model's time-stamps (especially when coupling to ADCIRC). Therefore, we need to allow for (linear) interpolation from input series onto the current time stamp of the NN model.
  • The rainfall input and the water surface elevation are currently stored in a single variable in the code (created through pandas.read_csv() call). The rainfall and elevation input variables must be separated because when coupling with ADCIRC, the values as well as the time stamps of the elevation input to the NN model must be modified, whereas those of the rainfall input remain unchanged.

Therefore, I propose we have something like the following:

class InputTS():
    def __init__(self, times, values):
        '''InputTS class constructor.'''
        self.times = times
        self.values = values
        self.current_index = 0

    def _getTimeInterval(self, t):
        '''Get the time interval of the current time stamp.'''
        return index

    def updateTimeInterval(self, t):
        '''Update the time interval by updating current_index.'''
        return None

    def interpolate(self, time):
        '''Interpolate input series onto NN model's current time.'''
        return value
@gajanan-choudhary gajanan-choudhary self-assigned this Oct 2, 2020
@gajanan-choudhary gajanan-choudhary added the enhancement New feature or request label Oct 2, 2020
@west5678
Copy link
Collaborator

west5678 commented Oct 2, 2020

I'm not against it. But I want to point out two things:

  1. pandas.read_csv essentially reads a spreadsheet and turn it into a pandas Dataframe. You can easily overwrite any column as you wish (whether it is rainfall or water level).
  2. I feel the current code is already a little complicated. We probably want to make it liter and more understandable by using standard libraries.

@gajanan-choudhary gajanan-choudhary linked a pull request Oct 7, 2020 that will close this issue
@gajanan-choudhary
Copy link
Member Author

We will continue to use pandas on the neural network side. The code ended up looking complicated because I prioritized development speed instead of keeping the code organized. I'll clean up the code at some point for sure.

Nevertheless I have added a few commits in #3 for this issue. This still adds the class on the lines of the code snippet I mentioned earlier, that is,

class InputTS():

and so on. This is not part of the NN model, but is a part of the AdcircNN class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants