NREL / SAM

System Advisor Model (SAM)
BSD 3-Clause "New" or "Revised" License
337 stars 140 forks source link

Tidal turbine power curve generation #1807

Closed mjprilliman closed 2 months ago

mjprilliman commented 2 months ago

Description

-Modify ssc equation generating tidal turbine power curves to have rated power based on capacity factor calculations rather than an input power -Show calculated power per rotor -For patch 2, using a fixed capacity factor of 0.3 in ssc; for release expose capacity factor as an additional UI input -todo for release: rename UI variables for improved clarity.

-See email for test materials

Type of change

Please delete options that are not relevant.

Checklist:

If you have added a new compute module in a SSC pull request related to this one, be sure to check the Process Requirements.

mjprilliman commented 2 months ago

The functionality looks good.

When I choose the option to design the tidal converter, I see both the tidal design inputs and the inputs for the “choose tidal converter from library” option. The power curve table is technically disabled, but it is so prominent that I think most users will not know to ignore it, in spite of the red text at the top of the page. I think this should be set up like a battery dispatch page where the radio buttons are in a separate top panel and control what appears in the bottom panel.

I can't replicate this. When I switch to the tidal turbine design I only see the design page.

mjprilliman commented 2 months ago

@cpaulgilman Are you testing this on a Mac? I'm noticing the mac and Linux tests failing on SAM here (although only the pull_request) and I wanted to make sure there's not any differences across platforms. Also are you checking out the same branch name in ssc (https://github.com/NREL/ssc/pull/1181)?

brtietz commented 2 months ago

@mjprilliman The distinguishing feature seems to be that the pull request builds are failing and the push builds are passing. The pull request builds are building against patch of ssc:

image

which seems incorrect in this case. I think it's safe to ignore it in this case and @dguittet and I can double check the CI scripts to fix this in the future.

mjprilliman commented 2 months ago

@mjprilliman The distinguishing feature seems to be that the pull request builds are failing and the push builds are passing. The pull request builds are building against patch of ssc:

image

which seems incorrect in this case. I think it's safe to ignore it in this case and @dguittet and I can double check the CI scripts to fix this in the future.

Thanks. I was looking for this yesterday but was looking in the wrong place. I had to rebase these branches to patch (manually, not using the git rebase command) if that is helpful in troubleshooting the CI.

dguittet commented 2 months ago

Ok I think the issue is that the PR workflow should also use the related ssc branch if it's available. I'll make that change.