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 pvsamv1 properties and setters for interdependent parameters #130

Merged
merged 10 commits into from
Jun 29, 2023

Conversation

Matthew-Boyd
Copy link
Collaborator

This includes properties and setters for all interdependent parameters, including:

  • system capacity
  • modules per string
  • number of strings (restricting to subarray1 only)
  • PV module model
  • inverter model

@Matthew-Boyd Matthew-Boyd added the enhancement New feature or request label May 11, 2023
@Matthew-Boyd Matthew-Boyd self-assigned this May 11, 2023
Copy link
Collaborator

@dguittet dguittet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thought I had was that it would be good to move a lot of the functionality in pv_design_utils.py and pv_inverter.py` into PySAM where it'll probably get a broader user base. Not something we can do with the current project, but I'll ask if it's appropriate for SAM core

@dguittet
Copy link
Collaborator

@Matthew-Boyd I added an assert for the PV system capacity in here: a6275cc

This now causes the test_hybrid_detailed_pv_only to fail.

@dguittet
Copy link
Collaborator

@Matthew-Boyd Looking at the latest change, I think it went in the wrong direction. The DetailedPVPlant needs to be able to use a system capacity.

Can you tell me what the input dictionary format for DetailedPVPlant should be? Where does the system_capacity input go?

IIRC, the input dictionary format for the simple PVPlant is different since there's no "tech_config" required.

@Matthew-Boyd
Copy link
Collaborator Author

@Matthew-Boyd Looking at the latest change, I think it went in the wrong direction. The DetailedPVPlant needs to be able to use a system capacity.

Can you tell me what the input dictionary format for DetailedPVPlant should be? Where does the system_capacity input go?

IIRC, the input dictionary format for the simple PVPlant is different since there's no "tech_config" required.

@dguittet
The dictionary key system_capacity_kw in the top level of the pv_config parameter for DetailedPVPlant was not being used before this last commit. The value for system capacity was only being taken from the tech_config sub-dict in pv_config.

The input dictionary format for the DetailedPVPlant, as provided in tech_config, is demonstrated in pvsamv1_basic_params.json, which is the same as the SAM UI code generation for "PySAM JSON". The system capacity is adjusted for coherence with other model parameters and set in the model in DetailedPVPlant::processed_assign()

I will add another test to ensure the system_capacity_kw top-level key value is used properly and in place of any value for system_capacity in tech_config.

@dguittet dguittet merged commit 3d53f84 into master Jun 29, 2023
1 of 3 checks passed
@camirmas camirmas mentioned this pull request Aug 11, 2023
4 tasks
@camirmas camirmas deleted the detailed_pv_props branch August 11, 2023 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants