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

fix: Fix issue with dataframe scenario indexing #1111

Merged
merged 1 commit into from
Oct 20, 2023

Conversation

jetuk
Copy link
Member

@jetuk jetuk commented Oct 19, 2023

Correction the look-up of dataframe data when using user combinations of scenarios.

Fixes #1110.

Correction the look-up of dataframe data when using user combinations of scenarios.

Fixes #1110.
@knoxsp
Copy link
Contributor

knoxsp commented Oct 20, 2023

I tried to re-run the test model i posted in the issue, and have had the following error:

AttributeError: 'pywr.parameters._parameters.DataFrameParameter' object has no attribute '_Parameter__values'

This error had also been occurring on all my models from version 1.22, and on master. I've had to revert to v1.21 to fix this issue.

@jetuk
Copy link
Member Author

jetuk commented Oct 20, 2023

Are you building with Cython 3?

@knoxsp
Copy link
Contributor

knoxsp commented Oct 20, 2023

>>> import cython
>>> cython.__version__
'0.29.36'

@knoxsp
Copy link
Contributor

knoxsp commented Oct 20, 2023

I can confirm that my larger model runs with this fix (applied to v1.21). I have not checked whether the correct scenario data is being used. Will report back.

@knoxsp
Copy link
Contributor

knoxsp commented Oct 20, 2023

Confirmed that the correct scenario data is being used. Checked manually with print statements in the value function of the DataFrameParamter class.

@jetuk
Copy link
Member Author

jetuk commented Oct 20, 2023

Do you need this back-porting to 1.21 as well?

@knoxsp
Copy link
Contributor

knoxsp commented Oct 20, 2023

Do you need this back-porting to 1.21 as well?

I have upgraded cython to the latest version (3.0.4) and re-run using this branch. It now works as expected. So 1.22 doesn't work with pre-cython 3.

I can use cython 3 so i don't need this porting back to 1.21.

@jetuk jetuk merged commit e6818a8 into master Oct 20, 2023
12 checks passed
@jetuk jetuk deleted the issue1110-dataframe-scenario-combinations branch October 20, 2023 12:42
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.

Multiple scenario_combinations resulting in an index error.
3 participants