[com4]: allow executing FlowPy from a different repo with cfg overwrite - #1298
Conversation
|
Coverage Impact ⬆️ Merging this pull request will increase total coverage on Modified Files with Diff Coverage (2)
🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
There was a problem hiding this comment.
Good addition - should work as a quick hack and not break any current usage of the script.
For a more permanent solution I would suggest at least to address the following points:
- instead of just returning
uidat every exit point of the function we can return sth. like a named tuple or dictionary with the uid and a function exit status, that lets the caller know if the function executed com4FlowPy successfully or did not run the model - because input was incorrect, result folders already exist --> see extra comment below - write a small test that confirms the expected behavior of the functionality for different scenarios
in addition we could use this to think about how we want to handle the situation of already existing results with the same uid, which is currently also not solved well. Maybe we could:
- add an additional check, that also looks if the existing result folder with the
uidis populated with valid resultFiles instead of just checking if the folder already exists (now this could also result from a previously started, but aborted model run) --> this would fix Issue #1135 - add an additional function parameter
overwritewhere the user can specify e.g.default--> don't run simulation if results with sameuidalready there;overwriteif sims should be performed any way
6bdcf91 to
52f01ef
Compare
ahuber-bfw
left a comment
There was a problem hiding this comment.
some minor changes --> see comments
a968f92 to
cacd41c
Compare
There was a problem hiding this comment.
schaut jetzt gut aus - 2-3 kleine Punkte noch:
- ich hab noch ein paar kleine Änderungen gepusht (fe1faa3)
- bitte für die neuen Funktionen im
in3Utils/fileHandlerUtilsnoch jeweils einen Test ins entsprechendetest_fileHandlerUtilsfile - evtl. die Doku noch anpassen für den neuen overwrite Parameter im .ini
dann passts von meiner Seite :)
59be2ad to
f0c2d55
Compare
ahuber-bfw
left a comment
There was a problem hiding this comment.
pytests laufen alle durch
hab noch die Doku angepasst (vlt. kannst du da nochmal kurz drüber schauen @PaulaSp3)
commits sind gesquasht und rebased
von mir aus ready
main features/changes:
- runCom4FlowPy.main() now callable with cfg overwrite
- function takes additional (optional) cfg parameter
- function now returns a dictionary containing
- uid (simHash)
- returnStatus on 'simulation performed' and 'resultOverwritten'
- return message
- added additional config parameter to manage overwrite behavior (e.g.
in case of already existing results with same simHash)
- default (stock behavior, do not re-run simulation if valid model
results for same config already exist)
- reRunAndOverwrite (perform simulation, regardless of existing
previous model results and delete existing)
- reRunAndBackup (perform simulation, backup existing results)
updated com4FlowPy documentation for changes introduced by PR#1298
Co-authored-by: Paula Spannring <95042192+PaulaSp3@users.noreply.github.com>
0ce240d to
b8981f1
Compare
|
standard tests ident (apart from snow glide test) |

update:
the return of the
runCom4FlowPy.main()function is a dictionary containing:a simulation is executed, when there are no files in the results folder (with the same uid as the simulation that should be done)
a flag
overwriteResultsis added,Trueresult files are created although they already exist with same uidadded test
PR Checklist
Please confirm before requesting review:
pytestlocally without failsConfirm before the final merge/rebase into master