Open boykovdn opened 1 year ago
Merging #61 (2cfb3de) into main (f151a98) will decrease coverage by
0.72%
. The diff coverage is50.00%
.
@@ Coverage Diff @@
## main #61 +/- ##
==========================================
- Coverage 69.45% 68.73% -0.72%
==========================================
Files 15 15
Lines 478 483 +5
==========================================
Hits 332 332
- Misses 146 151 +5
Impacted Files | Coverage Δ | |
---|---|---|
src/p2lab/__main__.py | 64.61% <50.00%> (-0.39%) |
:arrow_down: |
Thanks for this @boykovdn -- looks good. I'm thinking we could probably move in the direction of having this read in from a config file, as this will run the simulations with default arguments everytime. We can even use that to define the directory that docker mounts to print out results I would think!
I can happily take a look at this and build on this PR at some point, but if you have time and want to I can let you lead that :)
Nice idea, I'll maybe have a look and give it a shot soon!
Great! I'll leave this PR for the time being then -- thanks :)
Changed the main.py to run with a config file. I don't know if this is the best way to parametrise the container, and I couldn't test the full building + running because the container is built from the main branch directly 😅
I did try overriding the source in the container with the one from my local filesystem and it seemed to work, so hopefully it's fine!
@boykovdn ok this is great!
regarding the fact you clone on the fly, I think we can probably change it to just copy all the relevant files from the Dockerfile
directory into the container. Then you can test it properly :)
Also, an accidental consequence of using the configfile in the way you do is that you erase the command-line arguments! So maybe you can put that back in, and add a new arg called config
that is the path to the config file, which then reads all the relevant args. Then, a possible next step is that you can specify that as a docker build argument (i found this random page when googling), which makes sure to copy the config path into the container, and you can then run p2lab --config=$CONFIG_PATH
or however that's fed in!
After these two changes, then we're definitely good to merge :)
One less line of commands to run the simulation
Container runs server then checks that server is listening. Then runs p2lab.
The container might have to be changed at some point because the output probably won't be saved anywhere. I'd have to mount some persistent storage and save the simulation results there.