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

Upgrade SAM dep to 5.1 #517

Merged
merged 27 commits into from
Feb 24, 2025
Merged

Upgrade SAM dep to 5.1 #517

merged 27 commits into from
Feb 24, 2025

Conversation

ppinchuk
Copy link
Collaborator

@ppinchuk ppinchuk commented Feb 21, 2025

Upgrade SAM dep to 5.1

Breaking Changes

Deprecated computations using the TroughPhysicalProcessHeat PySAM module. In PySAM 6+, this module is completely removed. In PySAM 5.1, the module is importable, but it is impossible to set the solar_mult input (it's not a property of any of the input classes associated with TroughPhysicalProcessHeat. Try to search for it here). Unfortunately, the model also cannot execute without this input (see below), effectively making this code unusable. Therefore, we drop support for this computation.

Error:

Traceback (most recent call last):
  File "/reV/reV/generation/generation.py", line 734, in _run_single_worker
    out = cls.OPTIONS[tech].reV_run(
          ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/reV/reV/SAM/generation.py", line 580, in reV_run
    sim.run_gen_and_econ()
  File "/reV/reV/SAM/generation.py", line 735, in run_gen_and_econ
    super().run_gen_and_econ()
  File /reV/reV/SAM/generation.py", line 425, in run_gen_and_econ
    self.run()
  File "/reV/reV/SAM/generation.py", line 450, in run
    self.execute()
  File "/reV/reV/SAM/SAM.py", line 944, in execute
    raise SAMExecutionError(msg) from e
reV.utilities.exceptions.SAMExecutionError: PySAM raised an error while executing: "troughphysicalheat" for site 0
ERROR    reV.generation.generation:generation.py:1147 reV generation failed!
Traceback (most recent call last):
  File "/reV/reV/SAM/SAM.py", line 936, in execute
    self.pysam.execute()
Exception: trough_physical_process_heat execution error.
        solar_mult not assigned

@ppinchuk ppinchuk added dependencies Issues/pull requests related to a dependency p-high Priority: high labels Feb 21, 2025
@ppinchuk ppinchuk added this to the Update dependencies milestone Feb 21, 2025
@ppinchuk ppinchuk self-assigned this Feb 21, 2025
@ppinchuk ppinchuk linked an issue Feb 21, 2025 that may be closed by this pull request
@ppinchuk ppinchuk added breaking Breaks something in the API or config deprecation Add a deprecation warning to outdated functionality labels Feb 23, 2025
@codecov-commenter
Copy link

codecov-commenter commented Feb 23, 2025

Codecov Report

Attention: Patch coverage is 96.19048% with 4 lines in your changes missing coverage. Please review.

Project coverage is 87.69%. Comparing base (7400934) to head (d557d8c).

Files with missing lines Patch % Lines
reV/generation/base.py 70.00% 3 Missing ⚠️
reV/losses/scheduled.py 90.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #517      +/-   ##
==========================================
+ Coverage   87.59%   87.69%   +0.09%     
==========================================
  Files         122      121       -1     
  Lines       18354    18400      +46     
==========================================
+ Hits        16077    16135      +58     
+ Misses       2277     2265      -12     
Flag Coverage Δ
unittests 87.69% <96.19%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@ppinchuk ppinchuk marked this pull request as ready for review February 23, 2025 23:00
@ppinchuk ppinchuk merged commit a9d4523 into main Feb 24, 2025
14 checks passed
@ppinchuk ppinchuk deleted the pp/pysam_51 branch February 24, 2025 02:29
github-actions bot pushed a commit that referenced this pull request Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaks something in the API or config dependencies Issues/pull requests related to a dependency deprecation Add a deprecation warning to outdated functionality p-high Priority: high
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update to PySAM version 5.1
2 participants