search

HopkinsIDD / flepiMoP

The Flexible Epidemic Modeling Pipeline
https://flepimop.org
GNU General Public License v3.0
9 stars 4 forks source link

Document, Test, Repair `gempyor.parameters` Module #276

Open TimothyWillard opened 1 month ago

TimothyWillard commented 1 month ago

Very similar to GH-246 but for the gempyor.parameters module. I'm starting here since this seems to be one of the simpler modules that interact with the configuration files directly. I also suspect that I'll find some bugs/unexpected behaviors along the way (such as the time series data frame being dropped issue I've already discovered) so I'll leave open the possibility of fixing that here. If the repairs get too large in size I'll migrate that to a separate issue.

TimothyWillard commented 1 month ago

Methods that need to be removed on the next PR for this issue:

  1. picklable_lamda_alpha
  2. picklable_lamda_sigma
  3. get_pnames2pindex

See https://github.com/HopkinsIDD/flepiMoP/pull/277#discussion_r1702265655.

TimothyWillard commented 1 month ago

This was originally a comment in GH-277 in relation to the second insufficient dates ValueError in the constructor of the Parameters class. I'm not sure how to get to the second pathway to this error message.

  1. We subset the read in dataframe to ti to tf so if the dataframe goes from 2024-01-01 through 2024-01-05 and the given date range is 2024-01-02 through 2024-01-06 the dataframe's date range will be subsetted to 2024-01-02 through 2024-01-05 which is a repeat of the above.
  2. Because of the subsetting you can't provide anything except a monotonic increasing sequence of dates, pandas only allows subsetting on ordered date indexes so you'll get a different error.
  3. If you provide a monotonic increasing sequence of dates but 'reverse' ti and tf you get no errors (which I think is also bad) because the slice operation returns an empty dataframe with the right columns & index and the pd.date_range function only creates monotonic increasing sequences and 0 == 0.

If this is dead code then I think we should just remove it, but @saraloo and @jcblemai do y'all have any thoughts?

See: https://github.com/HopkinsIDD/flepiMoP/pull/277#discussion_r1701855332.