Open BryanHurst opened 4 years ago
They also want the ability to have the script just run everything in the workspace folder unless there is a DO.NOT.RUN file in the scenario folder.
Maybe have a --auto argument passed in to run in this way?
Actually, lets see if we can build this into the adsm executable as a command line argument instead of creating a separate script.
You'll notice that the entry point already accepts command line arguments for testing.
If we are going to be doing command line arguments anyways we might as well take advantage of that. @BryanHurst how do you feel about these command line arguments?
--scenarios-path [directory location] A file path to a folder containing all of the scenarios to run. If argument is not given the current ADSM working directory will be used.
--exclude-file [file name] A .txt file containing a list of scenarios to skip, one scenario name per line. If argument is not given all files will be run.
-v Verbose, would output a lot more print statements about what ADSM is doing.
So a run of ADSM where you wanted to loop through multiple scenarios would look something like:
ADSM.exe -auto --exclude-file "excluded scenarios list.txt" --scenarios-location "C:\Users\JohnDoe\Documents\ADSM Workspace\" -v
Note that only the placements of -auto
is important, all other arguments can be ordered however the user wants. Also, not all command line arguments need to be given at all, running ADSM.exe -auto
will work just fine (will run all scenarios from the default workspace).
Just those three arguments off the top of my head, we could add pretty much any option we wanted.
Due to the way that I am processing command line arguments the user will have to use -auto
not --auto
. I've updated my example runs in the above comment to reflect this.
@ConradSelig in one of my previous comments I pointed out that the entry point already accepts command line arguments. I didn't really expand on that. The ADSM.py file already uses the builtin argparse parsing system around the current line 81.
This would be the place to add extra arguments, and they allow for proper '--' syntax.
From there, you can have the entry point call a different management command instead of launching the server and viewer.
As for commands, after talking more with the client, here is what we'd like.
Make sure to read the argparse library docs as it will handle the filename and list capturing quite nicely.
OK argparse looks great, thanks for the tip.
Going to be honest in saying that I'm not sure exactly where the arguments in ADSM.py around line 81 come in. I tried to add the additional arguments in there and had no luck actually getting anything to happen with them.
I did find a different solution however, that being to add another management command.
ADSM.exe auto [options]
(just like runserver
or devserver
)
@BryanHurst Thoughts on this?
Regardless, I'm going to add the following options into your list above.
I'm going to make the default action to be that if both --run-scenarios
or --run-scenarios-list
and --run-all-scenarios
are specified to ignore --run-all-scenarios
and just run the specified scenarios.
Yes, my last comment mentioned that management commands are the way to go.
The arguments defined around line 81 are then checked around line 168. Depending on which argument is flagged, it calls the desired management command.
I do like your extra argument options.
As for argument mismatches, it is generally acceptable to throw an error when incompatible arguments are passed in. In that case you would want to raise a RuntimeError
I was just thinking that --run-all-scenarios
wouldn't server any purpose if we ignore it when specific scenarios are given.
I like the idea of throwing an error, I'll do that.
Should exclusion or inclusion be more prevalent?
i.e. should the command ADSM.exe auto --run-scenarios "Example" --exclude-scenarios "Example"
include or exclude "Example"?
We could also just bypass the problem entirely by throwing an error if a scenario is both specifically included and excluded.
Probably best to throw an error. We don't want situations where things happen that the user doesn't expect.
On Thu, May 21, 2020, 8:13 PM Conrad Selig notifications@github.com wrote:
Should exclusion or inclusion be more prevalent?
i.e. should the command ADSM.exe auto --run-scenarios "Example" --exclude-scenarios "Example" include or exclude "Example"?
We could also just bypass the problem entirely by throwing an error if a scenario is both specifically included and excluded.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NAVADMC/ADSM/issues/1001#issuecomment-632439633, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJAWBPUOCS6GXQ2HZOTHXDRSXNWNANCNFSM4M6MBJXQ .
I'm having trouble with excluding scenarios that have a file called "DO.NOT.RUN" as Windows does not understand the ".run" file type and the file is then excluded from os.listdir().
Do we really want this feature? With the ability to have a single file that lists scenarios to exclude don't we think that users will find that more useful?
I'm sure I can get it working if pushed - but I'm not sure it is worth the extra time is will take.
os.listdir seems to pick up the DO.NOT.RUN file just fine
Python doesn't care about file extensions at all.
Having a file to designate not running is a fairly common standard.
Also, just to be sure after looking at one of your examples again.
ADSM.exe auto --run-scenarios "Example"
is actually
ADSM.exe --run-scenarios "Example"
OK I got DO.NOT.RUN to work - we shouldn't be surprised it was my fault that it wasn't working.
I do currently have it as ADSM.exe auto --run-scenarios "Example
, but this must be where the arguments in ADSM.py (around line 81) come in, because we can automatically detect which management command to apply so the user doesn't have to specify it themselves! The puzzle pieces are coming together.
I'm going to add another argument because the code I'm using to run the simulations themselves already has it built in.
--max-iterations [value]
Pretty self explanatory. Maxes out the number of iterations any one simulation can run. I don't really see a case where you would use this, but implementation is so easy we might as well make it an option.
@BryanHurst Do we want any support for users possibly on Linux? My last impression is that we don't really support anything besides Windows...
Also, how much do we care about log files? I would be happy to store all of the log files from each iteration if we want, but just not saving them in the first place would be easier. I don't think either path would have much of an affect on performance.
Correct. Right now is windows only.
On Mon, Jun 1, 2020, 2:05 AM Conrad Selig notifications@github.com wrote:
@BryanHurst https://github.com/BryanHurst Do we want any support for users possibly on Linux? My last impression is that we don't really support anything besides Windows...
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NAVADMC/ADSM/issues/1001#issuecomment-636682810, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJAWBIPU737YDWMUEDKQGTRUNON5ANCNFSM4M6MBJXQ .
OK, I'm just going to dump logs for now in the interest of not drawing out this project for another couple weeks.
With that initial functionality should be ready to go! A few notes from me right off the bat, so don't get too excited - the work is not yet done:
We spoke briefly about removing the need for the keyword 'auto' in the command. However, I couldn't actually figure out how to get Django to recognize my own arguments - it seems to exit before it even gets to ADSM.py. I went ahead and committed my changes to ADSM.py regardless so you can take a look, but even the commands that were there before (like "--version") throw errors when I run ADSM.exe --version
, so I'm not really sure what is going on with management.call_command() flow.
The statement "Defaulting to Portable Workspace in this Installation" prints multiple times, the exact number seems to depend on a number of criteria: Once for each difference scenario being run, in addition to another print for each iteration inside each scenario. (So if we run two scenarios, one with 1 iteration and the other with 2 iterations, the statement prints 5 times). This is because each thread is importing ADSM/settings.py. I'm looking into a way to prevent this from printing because it may be confusing to users - especially if they are running many large scenarios that would cause 100's of these identical output statements.
I am worried that the path I'm using to point to the adsm_simulation.exe
may cause problems when built into the release version (mostly because I know nothing about said build process). @BryanHurst please take a look in auto.py at the first real code line of Simulation().run() that is declaring the sim_path
and let me know if you think this will cause a problem (I would give a line number but we both know how volatile those can be).
I've got a few optimizations I want to make - mostly just removing unneeded code at this point. I'm just noting this down so I don't forget.
We need some serious documentation seeing as we implemented 8 command line arguments, I'll write up something nice in markdown that we can give users.
After some further testing it seems that scenarios with a certain number of iterations are not completing. I'm not yet sure what the minimum requirement is, but I'm looking into fixing the problem now. It may also have something to do with the max-iterations argument. We'll see.
EDIT: Upon further review it seems my initial success was not so great. Simulations don't actually appear to be running. EDIT: Simulations are running! They don't appear to be saving the results however. Hmmm EDIT: OK I found out where the results are going (not into the database unfortunately). Now I just have to figure out how to get them into the database.
@missyschoenbaum I don't know if you want to spend time testing this or not, but so it is documented, here are the new command line options.
--run-all-scenarios
Passing this in will automatically run all scenarios in your Workspace unless they are otherwise excluded
--exclude-scenarios
With this, pass in a list of scenario names that you want to exclude from running (--exclude-scenarios "Scenario 1" "Scenario 2"
--exclude-scenarios-list
Pass in a file name of a file with a list of scenario names you want to exclude from running (one per line on the file)
--run-scenarios
If you don't want to run all scenarios, but just a small set, pass in a list of scenario names that you want to run (--run-scenarios "Scenario 1" "Scenario 2")
--run-scenarios-list
Similar to the exclude list, pass in a file name with a list of scenario file names you wish to run
--workspace-path
Pass in a path to a custom workspace folder if you don't want to use the same one the main ADSM Program uses. This can be useful if you want to drop a group of scenarios into a new folder and just use --run-all-scenarios
--output-path
Pass in a path to the location you would like and output log and iteration logs to be saved to.
--max-iterations
The max iterations any single scenario can run
Here is an example. From a command prompt or PowerShell open in the ADSM folder:
ADSM_Beta.exe --run-all-scenarios --exclude-scenarios "Sample Scenario" "Sample Scenario with Outputs" --output-path "E:\ADSM\2020-06-10_ADSM_Output"
Added a --quiet option to not be prompted about potentially losing unsaved work.
@BryanHurst when you say "Here is an example. From a command prompt or PowerShell open in the ADSM folder:", will that mean the application installation (in theory on your hard drive) or your application workspace (somewhere else, possibly on a portable drive)?
In the installation directory where the Adsm.exe lives.
On Fri, Jun 12, 2020, 10:05 AM Missy Schoenbaum notifications@github.com wrote:
@BryanHurst https://github.com/BryanHurst when you say "Here is an example. From a command prompt or PowerShell open in the ADSM folder:", will that mean the application installation (in theory on your hard drive) or your application workspace (somewhere else, possibly on a portable drive)?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NAVADMC/ADSM/issues/1001#issuecomment-643353552, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJAWBIF45PMLNIFGT6FH3TRWJG5RANCNFSM4M6MBJXQ .
Hit this after first run populated logs:
Could we turn off logging? - Missy and Conrad discussed, will keep logs and make some tweaks.
Another note that may be helpful. All the auto outputs are writing back into the install directory. Only the last scenario to complete had all iterations, plus the server output and the server error. The other scenarios had their individual iterations, and were specific to the count I passed (not as argument).
Is this where you wanted these output to go?
Another note just to capture. A blank map is created when user attempts to view results. Before user attempts to view, there is no map (no files within the Map folder). This is not a problem to me, but may want to confirm with other users.
@missyschoenbaum from your comment on #1002 the --quiet option is just "--quiet" not "--quietoption".
For logging, I am interested in the file already exists bug if you come across that one again.
For the log output location, each scenario creates its own folder in the current working directory unless you pass in the --output-path option. This is as expected.
@BryanHurst HA! I figured out the --quiet when Conrad and I were looking at it.
Logging is writing in to my install directory. I showed Conrad today and he is going to look at it and also the log exists error.
Logging writing into the install dir is expected unless the output path is defined.
On Fri, Jun 19, 2020, 11:13 AM Missy Schoenbaum notifications@github.com wrote:
@BryanHurst https://github.com/BryanHurst HA! I figured out the --quiet when Conrad and I were looking at it.
Logging is writing in to my install directory. I showed Conrad today and he is going to look at it and also the log exists error.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/NAVADMC/ADSM/issues/1001#issuecomment-646764415, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJAWBMD4FBFUO5IGNBGTNLRXOMFDANCNFSM4M6MBJXQ .
We want to add another command line argument that will dictate if logs are being created or not. Something along the lines of --write-logs
.
Logs will by default write to where the users terminal session is currently directed at, but can also be changed with the --output-path [path]
option.
This is the error you get if you have the application open. I can document that the app needs to be closed if needed.
UPDATE - I found I had the demo425.db stuck inside another folder. Not sure why. DIsregard this.
Humm, this is a new one. It appears to try to run something called demo425.db, which I probably had in my directory at some point. But, it is not there now, as shown in the File Explorer window. What history could it be looking at?
Warning prompt question - It asks if I am OK losing unsaved work. Does it really mean it is going to delete any previous saved results that may be in a scenario already?
@BryanHurst I am still getting the file already exists message. I just manually delete to work around.
@missyschoenbaum The prompt that you may lose unsaved work is talking about any changes you have made to the Currently Open Scenario in the ADSM program that haven't been saved back to the Scenario Database from Active Session.
However, it is also true that it will clear any saved results in all Scenarios as they are run.
I'm not having good luck today. Tried again after I removed demo425, and previous output files. It starts on scenario that has to be upgraded, which it does. Then I get an Index out of range. Will post 2 images to see whole process.
everything between these 2 images ran and gave OK message
Additional note: This scenario should not have validated. I can add in the last scenario that did complete. It looks like this one that failed didn't proceed through the DELETE FILES, then validate steps.
Is it possible we could change the warning text to also include they will be deleting all files?
@ConradSelig since so much is piling up on this one, I'll take it over including the switch to write logs or not.
In relation to the "demo425.db" bug, even though we found why it did this, the scenario discoverer shouldn't be recursing folders in the Workspace. This is also true in the main ADSM app when showing scenarios in the workspace for opening.
@missyschoenbaum on the list index out of bounds/validation/outputs not deleted bug.
The deleting outputs is only printed if there are outputs to be deleted.
It looks like the list index out of bounds is it actually from failing validation that isn't caught properly. I'm surprised that that scenario would not cause a crash in the main ADSM program when trying to validate. Can you confirm that RedoSample does properly validate and run from the GUI?
EDIT: I pushed a potential fix for this, so test before updating.
NOTE: This is now a new command line argument to specify if you want to store the logs from each scenario.
--store-logs
If not specified, the logs will be overwritten with each scenario run.
Added better detection of when ADSM is already running.
Referencing the "RedoSample" which does not validate. I am getting a new message.
Auto_output.txt says RedoSample passed validation
@missyschoenbaum similar to the list index out of range issue, this looks like the RedoSample scenario is not a valid scenario. This issue should also crash in the ADSM UI.
Does this scenario run properly in the UI?
@BryanHurst The scenario does not validate or run. I am more trying to understand what the expected process is so I can create appropriate documentation. Let me clarify my question on this. Do I need to have documentation that tells users not to attempt running scenarios that do not validate?
I'm just not sure how the main program is catching that error during validation without completely crashing.
It would be good for us to properly catch all the validation problems and not rely on the user to test each scenario first.
I just pushed a fix that will catch that specific problem in the auto runner.
It has been requested that we create a script that can be setup in a way to run multiple scenarios back to back.
This should be done in a way that it can work in Windows or Linux, but not require any setup (no dependencies). It may be nice to be able to call this from the UI in the future.
The script should either look at an input file for a list of scenarios or accept them as command line arguments.