Enhance use of batch scripts #133
Conversation
…impleSolar or SolarPySAM)
|
@misi9170 and @genevievestarke , at this example 8 works with the method of looking for open ports and chaining them through. I did find still it can happen you need to force a new port, but the new first_port variable in the runscript provides a single location to force this change. Before I replicate these changes to the other examples, would you mind to have a look over example 8, see if it runs for you and let me know if don't like anything? |
There was a problem hiding this comment.
@paulf81 I reviewed this pull request! The changes seemed split between Ex 08 and Ex 09, so I updated the input file for Ex 08 to make a hopefully comprehensive example. I also adjusted floris_standin.py and amr_wind_standin.py to hopefully act the same in the examples. Let me know if I broke something!
| source "$CONDA_PATH" | ||
| conda activate hercules | ||
| # Locate the scripts folder | ||
| SCRIPTS_DIR="../../scripts" |
There was a problem hiding this comment.
Are the forward slashes going to create issues for Windows users? This PR is not the first to use forward slashes in shell scripts, but just wondering if it's something we need to be thinking about
There was a problem hiding this comment.
if you run the script with bash though? I'm not sure,
There was a problem hiding this comment.
I think if a windows user is using wsl, or git bash should be fine, if not maybe we just need a windows version of these scripts? Is there a windows native equivalent of bash?
There was a problem hiding this comment.
Agreed, perhaps we should just be encouraging Windows users to use WSL... presumably, users could try to run things in the Command Prompt, and I'm not sure it would work.
|
Thanks @genevievestarke , I think I've carried the changes through the rest of the examples now |
| temp = read_amr_wind_input(amr_input_file) | ||
|
|
||
| # Check that helics_port is an integer | ||
| # If helics_port provided update with value | ||
| if helics_port is not None: |
There was a problem hiding this comment.
Do we need an else clause? Is the idea that helics_port can be provided by the user as an argument, or it can come from the input yaml?
There was a problem hiding this comment.
as written if helics_port is not provided, it doesn't over write the value already implied by the input yaml, but if there is no value there we'd need to do something, not sure which way we wanted to go
|
How confident are we that it is only processes that have "helics" in the name that cause the usual "Helics broker could not connect" (or similar) message? I had thought that some of the python calls can also cause this hangup, because they have also received the |
Not too confident! I've seen there are still some exceptions (at least on my machine) that cause the later helics federates to go to a higher port number. A nice thing about the new format is that there is single place to set the port number, so it's pretty quick to just try a new starting port in the event of trouble |
This draft pull requests starts the effort to use more draft scripts to automate certain start-up processes and avoid issues related to bad conda set ups and helics_port selections.
Proposed changes
scripts/find_and_kill_helics.shlooks to find and kill any open helics jobsactivate_conda.shsets up the CONDA environment within bashget_helics_port.shlooks for an open set of 10 consecutive ports, and assigns the first toHELICS_PORTWhen we feel good about this
hercules_runscript_CLcontroland similar to acceptHELICS_PORTas a command line argument and not something specified in input filesBefore completing