feat(geospatial): add alchemist-geospatial module for real-world climate data - #5556
feat(geospatial): add alchemist-geospatial module for real-world climate data#5556Wanes01 wants to merge 108 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5556 +/- ##
=========================================
Coverage 61.53% 61.53%
Complexity 14 14
=========================================
Files 2 2
Lines 78 78
Branches 4 4
=========================================
Hits 48 48
Misses 24 24
Partials 6 6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Tick the box to add this pull request to the merge queue (same as
|
4e73252 to
6da2fa4
Compare
DanySK
left a comment
DanySK
left a comment
There was a problem hiding this comment.
There are several critical issues that must be solved before I go ahead with the review. See comments.
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Why is this there? This induces a big problem with the entire dependency management of the simulator.
There was a problem hiding this comment.
There is a conflict between osmosis-osm-binary, which requires protobuf <= 3.25.5, and cdm-core (used to read formats such as NetCDF), which brings protobuf version 4.31.1.
I wasn't sure how to resolve the issue, so I resolved the conflict this way in both alchemist-maps and alchemist-full.
| for (entry in zip.entries()) { | ||
| // nothing to do if the entry is directory. | ||
| if (entry.isDirectory) continue | ||
|
|
There was a problem hiding this comment.
avoid inconsistent vertical spacing. No empty lines within methods. This applies throughout the codebase
| * as described in the file LICENSE in the Alchemist distribution's top directory. | ||
| */ | ||
|
|
||
| package it.unibo.alchemist.boundary.acquisition.utility |
There was a problem hiding this comment.
Why "acquisition"?
Also, in the simulator, we have a utils package. This file may be relocated in it.unibo.alchemist.boundary.utils
Also, this looks like a very general zip utility, why do we need to unpack zips? Can't we read them directly with the appropriate stream?
| resolutionStrategy { | ||
| eachDependency { | ||
| if (requested.group == "com.google.protobuf" && requested.name == "protobuf-java") { | ||
| useVersion("3.12.2") |
| repositories { | ||
| google() | ||
| mavenCentral() | ||
| maven("https://artifacts.unidata.ucar.edu/repository/unidata-all/") |
There was a problem hiding this comment.
Nope, we can't. Why do we need this? I'd rather republish this on Central. We cannot depend on artifacts that are not available on Maven Central.
There was a problem hiding this comment.
The cdm-core used in the project requires version >= 5.x, which is not published on Central. There is an old artifact on Central with version 4.3.22; I tried using that one, but it had problems reading the latest version of NetCDF (NetCDF4).
e7313e7 to
a01f7e6
Compare
…uil-in implementation
… SpatialExtrapolationStrategy
…tween spatially resolved values
…r with data download/caching needs to be added)
…e; move Copernicus inputs into an external JSON
…in RasterGrid implementations
…lize arguments validation
…reduce responsability of EagerGridSnapshots by extracting reusable functions to read and validate a NetCDF file
…nst but preserves the first one only
…with GridSnapshots
…udes in RasterGrid
a01f7e6 to
a42e045
Compare
|
3 participants
No description provided.