Skip to content
This repository was archived by the owner on Sep 11, 2023. It is now read-only.

Remove time_30 and time_5 #230

Closed
Tracked by #213
JackKelly opened this issue Oct 15, 2021 · 5 comments
Closed
Tracked by #213

Remove time_30 and time_5 #230

JackKelly opened this issue Oct 15, 2021 · 5 comments
Labels
enhancement New feature or request refactoring

Comments

@JackKelly
Copy link
Member

After #223 is implemented, the code should be able to handle any arbitrary sample rate. And, once we output each modality into a different NetCDF file, each NetCDF file will have a single sample period. So always refer to time as time and remove any code that talks about time_5 or time_30

@JackKelly JackKelly added enhancement New feature or request refactoring labels Oct 15, 2021
@flowirtz flowirtz moved this to Todo in Nowcasting Oct 15, 2021
@peterdudfield
Copy link
Contributor

This is covered in
#229

Repository owner moved this from Todo to Done in Nowcasting Oct 20, 2021
@peterdudfield
Copy link
Contributor

This is now merged, do shout if there is something you think is not done @JackKelly

@JackKelly
Copy link
Member Author

Cool beans. Do we still need these methods in nowcasting_dataset.config.model.Process:

  • seq_length_30_minutes()
  • seq_length_5_minutes()
  • history_minutes_divide_by_30()
  • forecast_minutes_divide_by_30()

@peterdudfield
Copy link
Contributor

peterdudfield commented Oct 20, 2021

Im doing them in #217

but i think we do, or at least some of them. the sequence thing is useful for making test data.

The validation on history minutes, could only apply to gsp data for example. Or maybe ive mis understood how it will be used

@JackKelly
Copy link
Member Author

cool beans, sounds good!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request refactoring
Projects
No open projects
Status: Done
Development

No branches or pull requests

2 participants