Fix AVMList clone between different models

[kbenne]

close #4033
close #4034

Pull request overview

• Fixes #ISSUENUMBERHERE (IF THIS IS A DEFECT)
• DESCRIBE PURPOSE OF THIS PULL REQUEST

Please read OpenStudio Pull Requests to better understand the OpenStudio Pull Request protocol.

Pull Request Author

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

• Model API Changes / Additions
• Any new or modified fields have been implemented in the EnergyPlus ForwardTranslator (and ReverseTranslator as appropriate)
• Model API methods are tested (in src/model/test)
• EnergyPlus ForwardTranslator Tests (in src/energyplus/Test)
• If a new object or method, added a test in NREL/OpenStudio-resources: Add Link
• If needed, added VersionTranslation rules for the objects (src/osversion/VersionTranslator.cpp)
• Checked behavior in OpenStudioApplication, adjusted policies as needed (src/openstudio_lib/library/OpenStudioPolicy.xml)
• Verified that C# bindings built fine on Windows, partial classes used as needed, etc.
• All new and existing tests passes
• If methods have been deprecated, update rest of code to use the new methods

Labels:

• If change to an IDD file, add the label IDDChange
• If breaking existing API, add the label APIChange
• If deemed ready, add label Pull Request - Ready for CI so that CI builds your PR

Review Checklist

This will not be exhaustively relevant to every PR.

• Perform a Code Review on GitHub
• Code Style, strip trailing whitespace, etc.
• All related changes have been implemented: model changes, model tests, FT changes, FT tests, VersionTranslation, OS App
• Labeling is ok
• If defect, verify by running develop branch and reproducing defect, then running PR and reproducing fix
• If feature, test running new feature, try creative ways to break it
• CI status: all green or justified

[kbenne]

what do you think @jmarrec?

[jmarrec]

@kbenne Can we make it protected/private? Or at least ignore in the bindings... I don't like the fact that this becomes public, as users shouldn't have any business creating orphaned ones.

[kbenne]

yeah. I can do that. note that even if you use AvailabilityManagerAssignmentList(const Loop& loop) the new AVM is still orphaned. The loop argument is actually only used to set the name of the AVM. This itself should probably be considered a bug.

OpenStudio/src/model/AvailabilityManagerAssignmentList.cpp

Line 409 in fa9a77b

this->setName(loop.name().get() + " AvailabilityManagerAssignmentList");

[jmarrec]

I suppose to some extent and to echo #4077 AvailabilityManagerAssignmentList doesn't really have any business having a name in the OSM since the user isn't meant to interact with it... It could be created at FT time.

[kbenne]

@jmarrec you are certainly welcome to add to the list of objects in #4077 to "un-name" If it is not user facing then I think it is fair game. For those that have non uuid names, and perhaps even those that do, I would anticipate a performance advantage.

[kbenne]

Certainly users will appreciate having nice names in the finished EnergyPlus idf. AVMlist named after the loop it belongs to etc.

[ci-commercialbuildings]

CI Results for 7438d0d:

• Linux:

  • OpenStudio-3.1.0-alpha+7438d0d79d-Linux.deb
  • OpenStudio-3.1.0-alpha+7438d0d79d-Linux.tar.gz

• Darwin:

  • OpenStudio-3.1.0-alpha+7438d0d79d-Darwin.dmg
  • OpenStudio-3.1.0-alpha+7438d0d79d-Darwin.tar.gz

• Windows:

  • OpenStudio-3.1.0-alpha+7438d0d79d-Windows.exe
  • OpenStudio-3.1.0-alpha+7438d0d79d-Windows.tar.gz