idaholab / moose

Multiphysics Object Oriented Simulation Environment
https://www.mooseframework.org
GNU Lesser General Public License v2.1
1.76k stars 1.05k forks source link

PorousFlow base level 2 material #6938

Closed WilkAndy closed 8 years ago

WilkAndy commented 8 years ago

@dschwen points out that

if (_phase_num >= _dictator_UO.numPhases())

is called a lot in the level2 materials. He suggests having a base class that holds _phase_num and does the check once. Let's do this once the backlog is merged, @cpgr .

cpgr commented 8 years ago

Could probably implement this in the constructor for PorousFlowFluidPropertiesBase that all of the fluid materials inherit from already? This is where _phase_num is read in from the input file

cpgr commented 8 years ago

But it is also called in rel perm materials, so probably want a base class for the base class for all of these materials!

WilkAndy commented 8 years ago

Yes, i think you're right, @cpgr (base class for base class). I don't want to do this right now because once everything is merged it'll be clearer exactly where to do it, and also we'll be able to more easily re-factor when we've got our test suite merged.

cpgr commented 8 years ago

Agree - lets leave it until then

dschwen commented 8 years ago

:+1:

WilkAndy commented 8 years ago

Also

If PorousFlowFluidPropertiesBase inherited from the base class I suggested above you could also have a string member _phase that contains the stringified phase number. That would get rid of the twenty Moose::stringify calls.

cpgr commented 8 years ago

Have you started this @WilkAndy? If not, I'll do it now

WilkAndy commented 8 years ago

No, i haven't started. i'm working on something else (not porous flow) presently,

a

cpgr commented 8 years ago

I've finished this, but will wait until #7010 is merged before I submit a PR so that I don't reintroduce the UO in the dictator name

WilkAndy commented 8 years ago

Hey, that's cool, thanks @cpgr . I'm really enjoying this collaboration.