Deprecate old Cubic EoS by dallan-keylogic · Pull Request #1519 · IDAES/idaes-pse

dallan-keylogic · 2024-11-04T17:01:32Z

Summary/Motivation:

The old cubic equation of state has not been updated to have correct formulae for partial molar Gibbs and Helmholtz free energy, and doesn't have any formulae for heat capacity or speed of sound. Since it's been made obsolete by the cubic EoS implemented through the Modular Property Framework, there's no reason to keep it around to confuse people.

Changes proposed in this PR:

Deprecate old Cubic EoS property block, parameter block, and initializer.
Deprecate BT Peng-Robinson class based on old Cubic EoS

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

codecov-commenter · 2024-11-04T18:48:50Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.04%. Comparing base (3fa6904) to head (4a15b6d).
Report is 20 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1519      +/-   ##
==========================================
- Coverage   77.05%   77.04%   -0.01%     
==========================================
  Files         384      384              
  Lines       62335    62338       +3     
  Branches    10222    10222              
==========================================
  Hits        48031    48031              
- Misses      11875    11879       +4     
+ Partials     2429     2428       -1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

andrewlee94

Looks OK to me - just to confirm, have you checked the examples to make sure there is nothing there that depends on this?

dallan-keylogic · 2024-11-05T14:37:53Z

Looks OK to me - just to confirm, have you checked the examples to make sure there is nothing there that depends on this?

~~That's what the CI tests are for.~~ Yes, I have checked, nothing in the examples uses CubicParameterBlock or BTParameterBlock.

bpaul4

Looks good, @dallan-keylogic.

docs/reference_guides/model_libraries/generic/property_models/ceos.rst

…hinx link

dallan-keylogic · 2024-11-13T16:48:49Z

Got a test failure on Python 3.10 Windows, completely unrelated to the PR.

================================== FAILURES ===================================
______________________________ test_parity_alamo ______________________________

alamo_surrogate = <idaes.core.surrogate.alamopy.AlamoSurrogate object at 0x0000020CEE61A7A0>
data_validation =     Bypass_Fraction  NG_Steam_Ratio  ...        N2            O2
0          0.171014        1.000000  ...  0.341778  2...4  ...  0.317834  3.530000e-20
19         0.262319        1.200000  ...  0.324708  3.670000e-20

[20 rows x 15 columns]

    @pytest.mark.unit
    def test_parity_alamo(alamo_surrogate, data_validation):
    
        with TempfileManager.new_context() as tf:
            # note - a failure 'The process cannot access the file because it is
            # being used by another process' could occur if an internal error
            # arises before the results file is closed inside the surrogate method
    
            # create and step into new temporary directory
            dname = tf.mkdtemp()
            filename = os.path.join(dname, "results.pdf")
>           figs = surrogate_parity(
                alamo_surrogate, data_validation, filename=filename, show=False
            )

idaes\core\surrogate\plotting\tests\test_alamo_plotting.py:117: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
idaes\core\surrogate\plotting\sm_plotter.py:206: in surrogate_parity
    return _parity(
idaes\core\surrogate\plotting\sm_plotter.py:249: in _parity
    fig = plt.figure()
C:\Miniconda\envs\idaes-pse-dev\lib\site-packages\matplotlib\pyplot.py:1027: in figure
    manager = new_figure_manager(
C:\Miniconda\envs\idaes-pse-dev\lib\site-packages\matplotlib\pyplot.py:550: in new_figure_manager
    return _get_backend_mod().new_figure_manager(*args, **kwargs)
C:\Miniconda\envs\idaes-pse-dev\lib\site-packages\matplotlib\backend_bases.py:3507: in new_figure_manager
    return cls.new_figure_manager_given_figure(num, fig)
C:\Miniconda\envs\idaes-pse-dev\lib\site-packages\matplotlib\backend_bases.py:3512: in new_figure_manager_given_figure
    return cls.FigureCanvas.new_manager(figure, num)
C:\Miniconda\envs\idaes-pse-dev\lib\site-packages\matplotlib\backend_bases.py:1797: in new_manager
    return cls.manager_class.create_with_canvas(cls, figure, num)
C:\Miniconda\envs\idaes-pse-dev\lib\site-packages\matplotlib\backends\_backend_tk.py:483: in create_with_canvas
    window = tk.Tk(className="matplotlib")
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <tkinter.Tk object .>, screenName = None, baseName = 'pytest'
className = 'matplotlib', useTk = True, sync = False, use = None

    def __init__(self, screenName=None, baseName=None, className='Tk',
                 useTk=True, sync=False, use=None):
        """Return a new top level widget on screen SCREENNAME. A new Tcl interpreter will
        be created. BASENAME will be used for the identification of the profile file (see
        readprofile).
        It is constructed from sys.argv[0] without extensions if None is given. CLASSNAME
        is the name of the widget class."""
        self.master = None
        self.children = {}
        self._tkloaded = False
        # to avoid recursions in the getattr code in case of failure, we
        # ensure that self.tk is always _something_.
        self.tk = None
        if baseName is None:
            import os
            baseName = os.path.basename(sys.argv[0])
            baseName, ext = os.path.splitext(baseName)
            if ext not in ('.py', '.pyc'):
                baseName = baseName + ext
        interactive = False
>       self.tk = _tkinter.create(screenName, baseName, className, interactive, wantobjects, useTk, sync, use)
E       _tkinter.TclError: Can't find a usable init.tcl in the following directories: 
E           C:/Miniconda/envs/idaes-pse-dev/Library/lib/tcl8.6 C:/Miniconda/envs/lib/tcl8.6 C:/Miniconda/lib/tcl8.6 C:/Miniconda/envs/library C:/Miniconda/library C:/Miniconda/tcl8.6.14/library C:/tcl8.6.14/library
E       
E       C:/Miniconda/envs/idaes-pse-dev/Library/lib/tcl8.6/init.tcl: couldn't read file "C:/Miniconda/envs/idaes-pse-dev/Library/lib/tcl8.6/init.tcl": No error
E       couldn't read file "C:/Miniconda/envs/idaes-pse-dev/Library/lib/tcl8.6/init.tcl": No error
E           while executing
E       "source C:/Miniconda/envs/idaes-pse-dev/Library/lib/tcl8.6/init.tcl"
E           ("uplevel" body line 1)
E           invoked from within
E       "uplevel #0 [list source $tclfile]"
E       
E       
E       This probably means that Tcl wasn't installed properly.

C:\Miniconda\envs\idaes-pse-dev\lib\tkinter\__init__.py:2299: TclError

During handling of the above exception, another exception occurred:

alamo_surrogate = <idaes.core.surrogate.alamopy.AlamoSurrogate object at 0x0000020CEE61A7A0>
data_validation =     Bypass_Fraction  NG_Steam_Ratio  ...        N2            O2
0          0.171014        1.000000  ...  0.341778  2...4  ...  0.317834  3.530000e-20
19         0.262319        1.200000  ...  0.324708  3.670000e-20

[20 rows x 15 columns]

    @pytest.mark.unit
    def test_parity_alamo(alamo_surrogate, data_validation):
    
>       with TempfileManager.new_context() as tf:

idaes\core\surrogate\plotting\tests\test_alamo_plotting.py:109: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
C:\Miniconda\envs\idaes-pse-dev\lib\site-packages\pyomo\common\tempfiles.py:263: in __exit__
    self.release()
C:\Miniconda\envs\idaes-pse-dev\lib\site-packages\pyomo\common\tempfiles.py:420: in release
    self._remove_filesystem_object(name)
C:\Miniconda\envs\idaes-pse-dev\lib\site-packages\pyomo\common\tempfiles.py:469: in _remove_filesystem_object
    shutil.rmtree(name, ignore_errors=not deletion_errors_are_fatal)
C:\Miniconda\envs\idaes-pse-dev\lib\shutil.py:750: in rmtree
    return _rmtree_unsafe(path, onerror)
C:\Miniconda\envs\idaes-pse-dev\lib\shutil.py:620: in _rmtree_unsafe
    onerror(os.unlink, fullname, sys.exc_info())
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

path = 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmp2gyo2l4n'
onerror = <function rmtree.<locals>.onerror at 0x0000020CF16B6B00>

    def _rmtree_unsafe(path, onerror):
        try:
            with os.scandir(path) as scandir_it:
                entries = list(scandir_it)
        except OSError:
            onerror(os.scandir, path, sys.exc_info())
            entries = []
        for entry in entries:
            fullname = entry.path
            if _rmtree_isdir(entry):
                try:
                    if entry.is_symlink():
                        # This can only happen if someone replaces
                        # a directory with a symlink after the call to
                        # os.scandir or entry.is_dir above.
                        raise OSError("Cannot call rmtree on a symbolic link")
                except OSError:
                    onerror(os.path.islink, fullname, sys.exc_info())
                    continue
                _rmtree_unsafe(fullname, onerror)
            else:
                try:
>                   os.unlink(fullname)
E                   PermissionError: [WinError 32] The process cannot access the file because it is being used by another process: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmp2gyo2l4n\\results.pdf'

C:\Miniconda\envs\idaes-pse-dev\lib\shutil.py:[618](https://github.com/IDAES/idaes-pse/actions/runs/11802661048/job/32878857667?pr=1519#step:7:619): PermissionError
============================== warnings summary ===============================

andrewlee94 · 2024-11-13T17:02:13Z

@dallan-keylogic That is a known ongoing but intermittent issue. The plotting tests really need to be revisited to see if there is anything we can actually test - at the moment they just test that the code executes with no failures, but occasionally it results in errors like this.

lbianchi-lbl · 2024-11-13T20:20:44Z

@dallan-keylogic for reference, the issue @andrewlee94 is referring to is #1453.

lbianchi-lbl · 2024-11-14T19:48:44Z

The Codecov upload failures are due to codecov/feedback#577, which should now be fixed.

dallan-keylogic added 3 commits November 1, 2024 12:17

cubic_deprecation

36c98b8

Deprecation tests and update docs

a37274a

run Black

907b822

dallan-keylogic and others added 3 commits November 4, 2024 14:16

remove deprecation of stateblock and derived class

543f793

remove unused import

134cc49

Merge branch 'main' into deprecate-old-ceos

eb41ff6

dallan-keylogic marked this pull request as ready for review November 4, 2024 20:00

dallan-keylogic requested review from agarciadiego, bpaul4 and dangunter as code owners November 4, 2024 20:00

dallan-keylogic requested a review from andrewlee94 November 4, 2024 20:00

dallan-keylogic self-assigned this Nov 4, 2024

dallan-keylogic added Priority:High High Priority Issue or PR backward-compat Affects backward compatibility deprecation Add deprecation to codebase and/or remove deprecated feature labels Nov 4, 2024

andrewlee94 approved these changes Nov 5, 2024

View reviewed changes

bpaul4 approved these changes Nov 5, 2024

View reviewed changes

Merge branch 'main' into deprecate-old-ceos

bce9a7d

lbianchi-lbl reviewed Nov 7, 2024

View reviewed changes

docs/reference_guides/model_libraries/generic/property_models/ceos.rst Outdated Show resolved Hide resolved

lbianchi-lbl and others added 2 commits November 7, 2024 13:49

Try converting plain text reference to preferred implementation to Sp…

171fecf

…hinx link

Merge branch 'main' into deprecate-old-ceos

0f26e01

agarciadiego approved these changes Nov 12, 2024

View reviewed changes

Merge branch 'main' into deprecate-old-ceos

4a15b6d

ksbeattie merged commit b0f9156 into IDAES:main Nov 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Deprecate old Cubic EoS#1519

Deprecate old Cubic EoS#1519
ksbeattie merged 10 commits intoIDAES:mainfrom
dallan-keylogic:deprecate-old-ceos

dallan-keylogic commented Nov 4, 2024

Uh oh!

codecov-commenter commented Nov 4, 2024 •

edited

Loading

Uh oh!

andrewlee94 left a comment

Uh oh!

dallan-keylogic commented Nov 5, 2024

Uh oh!

bpaul4 left a comment

Uh oh!

Uh oh!

dallan-keylogic commented Nov 13, 2024

Uh oh!

andrewlee94 commented Nov 13, 2024

Uh oh!

lbianchi-lbl commented Nov 13, 2024

Uh oh!

lbianchi-lbl commented Nov 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Comments

Conversation

dallan-keylogic commented Nov 4, 2024

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

Uh oh!

codecov-commenter commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

andrewlee94 left a comment

Choose a reason for hiding this comment

Uh oh!

dallan-keylogic commented Nov 5, 2024

Uh oh!

bpaul4 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dallan-keylogic commented Nov 13, 2024

Uh oh!

andrewlee94 commented Nov 13, 2024

Uh oh!

lbianchi-lbl commented Nov 13, 2024

Uh oh!

lbianchi-lbl commented Nov 14, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov-commenter commented Nov 4, 2024 •

edited

Loading