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

BUGFIX storage term for timeIdx 1 with DRSDT #772

Closed
wants to merge 2 commits into from

Conversation

totto82
Copy link
Member

@totto82 totto82 commented Dec 15, 2022

The bug was introduced with the new tpfa linarizer and effect all cases with DRSDT /DRSTCON/DRVDT

I make it draft since it does not go well along with the current storageCache feature which seems to only be partly used by the new linearizer.

Fixes issue reported in OPM/opm-simulators#4318

@totto82
Copy link
Member Author

totto82 commented Mar 24, 2023

I think this issue should be fixed before the release. @atgeirr @hnil

@totto82 totto82 added this to the Release 2023.04 milestone Mar 29, 2023
@hnil
Copy link
Member

hnil commented Mar 31, 2023

I do not have full overview of the features. But I think it probably will be best to enforce cached and update them at end of time step. In principle eclproblem do not allow calculating IQ at anything else than timeIdx=0. I made separate change reintroducing all but have_stached in the tpfa linearizer which make.

@totto82
Copy link
Member Author

totto82 commented Mar 31, 2023

I guess you refer to #797 So your suggestion is to merge #797 instead. Did you try #797 for the case reported in OPM/opm-simulators#4318?

@akva2
Copy link
Member

akva2 commented Apr 13, 2023

should this hold back rc1?

@bska
Copy link
Member

bska commented Apr 13, 2023

should this hold back rc1?

In my opinion, no. It's still marked as "draft" and there's been no development in the past two weeks. As far as I'm concerned we should create RC1 without it. If we happen to get to a point where this is resolved we might consider back-porting the fix.

@totto82
Copy link
Member Author

totto82 commented Apr 13, 2023

We have a workaround (use the old linearizer) so no. I will remove the tag. I hoped that adding the tag would boost some activity in fixing this. I will remote the tag.

@totto82 totto82 removed this from the Release 2023.04 milestone Apr 13, 2023
@atgeirr
Copy link
Member

atgeirr commented May 31, 2023

Closing in favour of #806 and #808.

@atgeirr atgeirr closed this May 31, 2023
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.

None yet

5 participants