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

Enhancing tests for design module's total cost #173

Merged
merged 6 commits into from
Aug 21, 2024

Conversation

nRiccobo
Copy link
Collaborator

Updated several design tests to include some kind of "test_total_cost". A simple test like this should capture a snapshot of the design module's total cost for given a base case. Further, this will aid in tracking if any logic, default costs, or cost algorithms change between additions.

Additionally, I addressed some potential if-statements matching issues in electrical_design and mooring_system_design that @rafmudaf noticed in #171.

…to electrical design for hvac, hvdc-monopole, hvdc-bipole. Included .upper() to various if-statements checking HVDC or HVAC selection.
… Included a .lower() to anchor_type and mooring_type if-statements
@nRiccobo nRiccobo added enhancement Enhancement to an existing feature infrastructure Features and requests related to model infrastructure labels Aug 16, 2024
@nRiccobo nRiccobo requested a review from RHammond2 August 16, 2024 17:40
@nRiccobo nRiccobo self-assigned this Aug 16, 2024
@nRiccobo nRiccobo mentioned this pull request Aug 16, 2024
@rafmudaf
Copy link
Contributor

Thanks @nRiccobo, nicely done.

Just a comment - using the form "key" in self.config["key"].lower() instead of self.config["key"].lower() == "key" means that ORBIT will support inputs that have the right parameter somewhere within the string such as "mooring_system_design": {"mooring_type": ["SemiTautttt"]}. That kind of flexibility can be really nice, but it's not the usual so I just wanted to point it out in case it wasn't intentional. You'll also have to be careful not to add a valid config value that appends something to an existing value like "HVDC_aluminum" etc for the cable.

Also, you could consider making all the inputs lower case in load_config and then use lower case in all the if-statements, and this would remove .lower() everywhere else. It's probably a painful amount of work to catch this everywhere, though.

Just my two cents. Thanks for addressing my other comment @nRiccobo!

Copy link
Collaborator

@RHammond2 RHammond2 left a comment

Choose a reason for hiding this comment

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

Thanks for getting these additional checks in place!

I think you opened up a small can of worms with some of input handling methods, otherwise the only thing I'd add is a total design cost for the default fixed and floating scenarios to ensure all the costs are double checked.

ORBIT/phases/design/electrical_export.py Outdated Show resolved Hide resolved
ORBIT/phases/design/electrical_export.py Outdated Show resolved Hide resolved
ORBIT/phases/design/mooring_system_design.py Outdated Show resolved Hide resolved
@@ -209,7 +213,7 @@ def calculate_line_length_mass(self):
+ self.rope_length * rope_mass_per_m
) / 1e3 # tonnes

elif self.mooring_type == "TLP":
elif self.mooring_type == "Tlp":
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see how this is now annoying because there are many forms of standardization this could take.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah... Never a one size fits all, heh.

Copy link
Collaborator

@RHammond2 RHammond2 left a comment

Choose a reason for hiding this comment

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

We could probably think a bit more on the capitalization of SemiTaut, TLP, etc., but I think it's good to go as-is.

@nRiccobo nRiccobo merged commit a2a5d93 into WISDEM:dev Aug 21, 2024
7 checks passed
@nRiccobo nRiccobo deleted the tests/design-costs branch August 21, 2024 15:51
@nRiccobo nRiccobo added this to the v1.1 Release milestone Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement to an existing feature infrastructure Features and requests related to model infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants