Skip to content

Replace JSONcpp with RapidJSON#664

Merged
sjanzou merged 3 commits into
developfrom
RapidJSON
Nov 19, 2021
Merged

Replace JSONcpp with RapidJSON#664
sjanzou merged 3 commits into
developfrom
RapidJSON

Conversation

@sjanzou

@sjanzou sjanzou commented Nov 18, 2021

Copy link
Copy Markdown
Collaborator

run export_config, defaults, ssc tests and landbosse

run export_config, defaults, ssc tests and landbosse
@sjanzou

sjanzou commented Nov 18, 2021

Copy link
Copy Markdown
Collaborator Author

Goes with SAM pull request 782 NatLabRockies/SAM#782

@brtietz brtietz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When I try to run landbosse, it just spins forever. Pulling things up in the debugger it looks like too many escape characters have been added to the file path in the input JSON:

| input_dict_as_text | "{'weather_file_path':'C:\\Users\\bmirletz\\source\\repos\\sam_dev\\sam\\deploy\\wind_resource\\WY Southern-Flat Lands.srw','interconnect_voltage_kV':137.0,'distance_to_interconnect_mi':0.0,'depth':1.0... | const std::string &

Unrelated to the changes in this PR, is there a way to introduce more error checking into the for loop on line 285 of call_python_module_windows? It seems like either:
(1) There should be some maximum number of cycles that this should run before exiting to an error. AND/OR
(2) It should be checking std::err in addition to std::out to pick up on errors like this.
I'm fine to solve that in a patch, but this infinite loop makes for some fun debugging...

@brtietz

brtietz commented Nov 18, 2021

Copy link
Copy Markdown
Collaborator

It looks like the clipboard removed some of the backslashes. Here's a screenshot of the debugger that shows (what I suspect is) the issue:

image

@cpaulgilman cpaulgilman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In my test of LandBosse from the UI, SAM crashes when I check Enable Land-Based Balance of System Cost Model on the Installation Costs page for a Wind / Single Owner case.

@brtietz

brtietz commented Nov 18, 2021

Copy link
Copy Markdown
Collaborator

Update after some additional testing: if I install landbosse from develop and check the box, it works just fine. If I remove deploy/runtime/python-3.7.6 and attempt to re-install from this branch, SAM crashes. At least the behavior is consistent with Paul!

@sjanzou

sjanzou commented Nov 18, 2021 via email

Copy link
Copy Markdown
Collaborator Author

@sjanzou

sjanzou commented Nov 18, 2021 via email

Copy link
Copy Markdown
Collaborator Author

@sjanzou

sjanzou commented Nov 18, 2021 via email

Copy link
Copy Markdown
Collaborator Author

@sjanzou

sjanzou commented Nov 18, 2021 via email

Copy link
Copy Markdown
Collaborator Author

@sjanzou

sjanzou commented Nov 18, 2021

Copy link
Copy Markdown
Collaborator Author

It looks like the clipboard removed some of the backslashes. Here's a screenshot of the debugger that shows (what I suspect is) the issue:

image

None of that code was touched for this pull request... Is landbosse working any differently from the 2020.11.29.r2 release?

@cpaulgilman

Copy link
Copy Markdown
Collaborator

Paul, Are you building from develop or this pull request? Both will crash along with 2020.11.29.r2 if you delete the deploy/runtime/python-3.7.6 folder and then attempt to rerun landbosse...

I built from the RapidJSON branch for both SAM and SSC. The runtime folder has a python folder with install_python.ps1, install_python.sh, landbosse.json, and python_config.json, which I guess are the files SAM uses to install python. There is no python-3.7.6 folder. (If I change the name of the folder from python to python-3.7.6 and start SAM I get a message box "Unhandled unknown exception; terminating the application.")

@sjanzou

sjanzou commented Nov 18, 2021

Copy link
Copy Markdown
Collaborator Author

Paul, Are you building from develop or this pull request? Both will crash along with 2020.11.29.r2 if you delete the deploy/runtime/python-3.7.6 folder and then attempt to rerun landbosse...

I built from the RapidJSON branch for both SAM and SSC. The runtime folder has a python folder with install_python.ps1, install_python.sh, landbosse.json, and python_config.json, which I guess are the files SAM uses to install python. There is no python-3.7.6 folder. (If I change the name of the folder from python to python-3.7.6 and start SAM I get a message box "Unhandled unknown exception; terminating the application.")

Here is my folder after doing a clean build with develop branches of lk and wex and RapidJSON branches of ssc and SAM
image

Then Wind->Single Owner -> enable landbosse
image

The issue with crashing was due to the python files that were checked into the runtime/python folder.
I added a reset folder with the intial install files so that SAM does not crash - again unreleated to this pull request.

The python.config file (if no runtime/python/python-3.7.6 folder) should look like
image

After landbosse enabled and 1200 python files and 285MB installated into the runtime/python/python-3.7.6 folder, the the python.config file should look likeL
image

@sjanzou

sjanzou commented Nov 18, 2021

Copy link
Copy Markdown
Collaborator Author

It looks like the clipboard removed some of the backslashes. Here's a screenshot of the debugger that shows (what I suspect is) the issue:

image

working with last commit to SAM... should be ready to go

@cpaulgilman

Copy link
Copy Markdown
Collaborator

LandBOSSE works for me now. Sorry that had to come up as part of this PR! ;)

@sjanzou

sjanzou commented Nov 19, 2021

Copy link
Copy Markdown
Collaborator Author

LandBOSSE works for me now. Sorry that had to come up as part of this PR! ;)

@cpaulgilman, that was great! Gave us a headstart on testing during lockdown!

@sjanzou

sjanzou commented Nov 19, 2021

Copy link
Copy Markdown
Collaborator Author

When I try to run landbosse, it just spins forever. Pulling things up in the debugger it looks like too many escape characters have been added to the file path in the input JSON:

| input_dict_as_text | "{'weather_file_path':'C:\Users\bmirletz\source\repos\sam_dev\sam\deploy\wind_resource\WY Southern-Flat Lands.srw','interconnect_voltage_kV':137.0,'distance_to_interconnect_mi':0.0,'depth':1.0... | const std::string &

Unrelated to the changes in this PR, is there a way to introduce more error checking into the for loop on line 285 of call_python_module_windows? It seems like either: (1) There should be some maximum number of cycles that this should run before exiting to an error. AND/OR (2) It should be checking std::err in addition to std::out to pick up on errors like this. I'm fine to solve that in a patch, but this infinite loop makes for some fun debugging...

Addressed in commit fc3d2ec

@sjanzou sjanzou merged commit 3513df8 into develop Nov 19, 2021
@sjanzou sjanzou deleted the RapidJSON branch November 19, 2021 05:09
@dguittet

Copy link
Copy Markdown
Collaborator

@sjanzou @cpaulgilman Great job adapting the LandBOSSE workflow's Python Installation from JSONCpp to RapidJSON! Looks like there were unexpected snags with writing and reading the JSON config files. And also re-installing Python after a manual deletion.

Apologies, I should have documented how this Python installation works, and how to adapt it! Can do that after the release. Anything else that came up while you were struggling through this obtuse, undocumented workflow that I should add to the documentation?

@brtietz brtietz added this to the SAM Fall 2021 Release milestone Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants