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

Add option for solving the energy equation sequentially #5854

Open
wants to merge 19 commits into
base: master
Choose a base branch
from

Conversation

totto82
Copy link
Member

@totto82 totto82 commented Jan 8, 2025

  • adds flow_blackoil_temp simulator that solves energy equation sequentially
  • activated by the TEMP keyword (THERMAL is still used for fully implicit)
    Depends on Implement TEMP option opm-common#4411

@totto82 totto82 force-pushed the addTEMP branch 5 times, most recently from 38f4c4f to 629c813 Compare January 10, 2025 08:17
@totto82
Copy link
Member Author

totto82 commented Jan 10, 2025

jenkins build this opm-common=4411 please

@totto82
Copy link
Member Author

totto82 commented Jan 10, 2025

@akva2 I dont build with HDF5 on the machine I am working on so I have being misusing jenkins for some testing without any sucsess. It seems to complain about some missing serializeOp(). I have tried to add them accordingly based on my understanding, but it keeps complaining. 1) Any hints of what I am missing? 2) Would adding a test for the temperatureModel serialization in test_RestartSerialization help detecting the issue that causes the failure related to HDF5?

@akva2
Copy link
Member

akva2 commented Jan 10, 2025

Well, you are on to it, but fundamentally you need to add serializeOp to the types your are serializing, in particular the EnergyVectors in storage1_ and the intQuants. But first a fundamental question:

I'm surprised that you need to serialize any of this. The only things you need to serialize are dynamic state that will not be recreated when we start a new report step and which is not derived from the primary solution. It looks to me like both of these are recreated from simulator state in beginTimeStep(), hence none of the serialization should be required, and the "fix" is to remove the serialization of the TemperatureModel in FlowProblem.

As an example, the only thing we serialize from the PVT classes (ie the material law manager) are the hysteresis parameters since these are dynamic in nature.

@totto82
Copy link
Member Author

totto82 commented Jan 10, 2025

and the "fix" is to remove the serialization of the TemperatureModel in FlowProblem

Does that mean that I need to remove these

template <class Restarter>
void serialize(Restarter&)
{ /* not implemented */ }

template
void deserialize(Restarter&)
{ /* not implemented */ }

The serializeOp is already commented

    //template<class Serializer>
    //void serializeOp(Serializer& serializer)
    //{
    //    serializer(intQuants_);
    //    serializer(storage1_);
    //}

@akva2
Copy link
Member

akva2 commented Jan 10, 2025

only serializeOp is used with flow. the other serialize/deserialize combo is an opm-models mechanism that is not in use in flow, but you need the functions there for the code to compile as they are called by simulator.hh.

you can just remove the implementation of serializeOp. but as I said you have to remove the serializer call for temperature model in FlowProblem's serializeOp(). that's the call site where you are telling it to call serializeOp in the temperature model.

@totto82
Copy link
Member Author

totto82 commented Jan 10, 2025

Arg.. I didn't notice that I had added it in FlowProblem. Thanks!

@totto82 totto82 force-pushed the addTEMP branch 2 times, most recently from 986459a to de11bb3 Compare January 15, 2025 13:48
@totto82 totto82 marked this pull request as ready for review January 16, 2025 08:10
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.

3 participants