-
Notifications
You must be signed in to change notification settings - Fork 78
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
Issue626 store results #629
Issue626 store results #629
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @HWalnum. Please see my inline comments and address. Also, can you add a note to the releasenotes.md
under the backwards compatible section? And can you re-target this PR to ibpsa:issue626_storeResults
instead of ibpsa:master
?
''' | ||
|
||
name = "results" | ||
# get results_json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To reduce code duplication, I suggest abstracting the creation of a results.json structure in a separate function for all fields that are common with what is submitted to dashboard, defined here https://github.com/ibpsa/project1-boptest/blob/master/testcase.py#L1143. Then, post_results_to_dashboard
and store_results
can share use of that function to build an initial json and add additional fields as-needed (if any) for submission to dashboard or writing to file, respectively.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I proposed a solution. However, I had some trouble with submitting results to the dashboard, with both previous and updated solution, so I have not checked that it works properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HWalnum I'll take a look, but looks like you got a couple results submitted for BESTEST Hydronic Heat Pump under the account "hwaln" on 3/8/2024 at https://dashboard.dev.boptest.net/.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @dhblum. I think those are from some extra tests I did with the service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok actually I'm having some trouble submitting to the dashboard too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And Ok I see. I'm meeting with Kyle tomorrow morning for a few things and he can help work out what's going on.
testcase.py
Outdated
"emulatorName": self.get_name()[2]['name'], | ||
"controlStep": str(self.get_step()[2]), | ||
"forecastParameters": {}, # for future use to store used parameters? | ||
"measurementParameters": {}, # for future use to store used parameters? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have an initial implementation described here: #514, but ran into an issue for truly accurate tracking as described at the end of that issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. I support the last proposed solution. But I guess this could be implemented first anyway?
Noting this is for #626. |
This is looking good. I will merge this to the staging branch and make a few adjustments there. |
@dhblum. Here is a first version of implementation of automatic storing results.
Please comment, both on the implementation and which parameters should be stored in results.json