Closed pjavanrood closed 1 month ago
Coverage Report
Tests | Skipped | Failures | Errors | Time |
---|---|---|---|---|
640 | 0 :zzz: | 0 :x: | 0 :fire: | 19.903s :stopwatch: |
I agree with Viktor. Let's merge the code to have just one repo. Having a separate repo and using it as a submodule adds unnecessary complexity. I was always under the impression that using a separate repo was for easy testing, but that we'll be integrating all code inside the main caribou repo.
Thank you both @vGsteiger and @engshahrad for your comments, as you both mentioned, I have added the Go source code as well, and we will keep everything in the same repo. I'm sorry for the confusion.
I am not very familiar with the Go language, so my review primarily focuses on the overall structure and its similarity to the Python implementation. I was able to successfully run the Go solver on my end, and I can confirm that the results are very similar to those of the Python solver. However, I did spot some minor issues, including one potential mistranslation in the carbon calculator.
Additionally, I was wondering if it's possible to modify the constant variables to load directly from constants.py. This would help avoid the need to update both the constants file and the individual Go file separately, improving maintainability. If you believe this is feasible and worthwhile, could you create an issue for this? There's no need to include this change in the current PR.
I also noticed that almost every Go function has a corresponding unit test, except for inputmanager.go. This might have been overlooked.
Overall, great work!
Thanks for your review, Daniel! I addressed the changes you mentioned and also created an issue for automatically syncing constants between Python and Go (#291 )
Moving Monte-Carlo Simulation to Go, closes:
This PR has two main components:
caribou-go/
GoDeploymentMetricsCalculator
caribou-go/
mirrors the functionality of theSimpleDeploymentMetricsCalculator
.The Go source code is compiled into a shared library, allowing it to be called from Python. The integration between Go and Python is managed through the
GoDeploymentMetricsCalculator
.Data is passed between the Python and Go components using named pipes. On the Python side, this communication is handled by
GoDeploymentMetricsCalculator
, while on the Go side, it's managed inmain/deploymentcalculator.go
.