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

Resolve "Adjustments using adjust require the input data of the control period to have the same size for the time dimension" #67

Conversation

btschwertfeger
Copy link
Owner

The discussion on #65 has shown an error that occurs, when using a workaround to adjust a data set based on control period data with differently shaped time-dimensions.

  1. So the first issue that this PR is addressing is to enable to use differently sized control period data for the adjustment. From now on, the time dimension of the modeled data of the control period must not have the same length as the time dimension of the reference data of the control period.

  2. The main issue that the mentioned discussion is about, is a strange result when applying the methods to un-evenly shaped data. The problem is described in more detail in How to deal with obs and simh if they have different time lengths? #65 (reply in thread). So to fix this, we now interpolate the cumulative distribution functions of the modeled data of the control period to the value range of the passed reference data.

Since the second issue was not present in this package, as it was only visible when just copying or hacking around, a single feature PR is sufficient, no further issues must be created.

@btschwertfeger btschwertfeger added the Feature New feature or request label Mar 8, 2024
@btschwertfeger btschwertfeger added this to the Upcoming Release milestone Mar 8, 2024
@btschwertfeger btschwertfeger self-assigned this Mar 8, 2024
Copy link

codecov bot commented Mar 8, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 97.14%. Comparing base (1d7e0b8) to head (a4e0627).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #67      +/-   ##
==========================================
- Coverage   98.08%   97.14%   -0.95%     
==========================================
  Files           7        7              
  Lines         261      280      +19     
==========================================
+ Hits          256      272      +16     
- Misses          5        8       +3     
Flag Coverage Δ
unittests 97.14% <85.71%> (-0.95%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
cmethods/distribution.py 98.03% <100.00%> (+0.03%) ⬆️
cmethods/core.py 92.45% <84.21%> (-4.77%) ⬇️

@btschwertfeger
Copy link
Owner Author

@Pan-Yuxian have a look; could you please try this change set on your data?

@Pan-Yuxian
Copy link

@Pan-Yuxian have a look; could you please try this change set on your data?看一看;您能否在数据上尝试此更改集?

Thank you @btschwertfeger !
I have modified the source code according to the code you provided and I can now successfully get the corrected plot.

@btschwertfeger btschwertfeger merged commit 15f78c1 into master Mar 10, 2024
35 checks passed
@btschwertfeger btschwertfeger deleted the 66-adjustments-using-adjust-require-the-input-data-of-the-control-period-to-have-the-same-size-for-the-time-dimension branch March 10, 2024 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature or request
Projects
None yet
2 participants