Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BRT #170

Merged
merged 37 commits into from
Oct 17, 2024
Merged

BRT #170

merged 37 commits into from
Oct 17, 2024

Conversation

gottacatchenall
Copy link
Contributor

It's BRT, but real this time

@gottacatchenall
Copy link
Contributor Author

@jmlord This almost works, but we are having issues with permissions where we can't write to the /tmp dir, which ArchGDAL.jl has to do to save geotiffs

@jmlord
Copy link
Contributor

jmlord commented Sep 20, 2024

we can't write to the /tmp dir, which ArchGDAL.jl has to do to save geotiffs

Let me know if 5e13462 solves your problems.
Building here: https://github.com/GEO-BON/bon-in-a-box-pipelines/actions/runs/10962572212

@@ -7,6 +7,9 @@ ENV JULIA_DEPOT_PATH="/julia_depot/"
# Pre-compiling Julia dependencies
RUN julia -e 'pwd(); using Pkg; \
Pkg.add.(["SpeciesDistributionToolkit", "Dates", "Clustering", "JSON", "CSV", "DataFrames", "StatsBase", "EvoTrees", "MultivariateStats" ]); \
Pkg.instantiate();'
Pkg.instantiate(); Pkg.precompile()'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to keep the precompile step since every pipeline is running in its own environment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

long story shart: everything is precompiled in /julia_depot for now, and the main script here activates /julia_depot as the project dir

scripts/SDM/BRT/fitBRT.jl Outdated Show resolved Hide resolved
scripts/SDM/BRT/fitBRT.jl Outdated Show resolved Hide resolved
scripts/SDM/BRT/fitBRT.jl Outdated Show resolved Hide resolved
@tpoisot
Copy link

tpoisot commented Sep 21, 2024

When this works I can also add a quick bootstrap uncertainty - @jmlord / @JoryGriffith what do you think?

@gottacatchenall
Copy link
Contributor Author

gottacatchenall commented Sep 23, 2024

this makes a half decent map with the parameters below. (you'll have to run ./dev-server build from the pipeline engine, it takes like 10 mins but only has to run once)

input yaml
data>GBIFHeatmapFromSTAC.yml@130|taxa: plants
data>loadFromStac.yml@119|mask: null
data>loadFromStac.yml@119|stac_url: https://stac.geobon.org
data>loadFromStac.yml@139|bbox:
  - -2316297
  - -1971146
  - 1015207
  - 1511916
data>loadFromStac.yml@139|mask: null
data>loadFromStac.yml@139|stac_url: https://stac.geobon.org/
data>loadFromStac.yml@139|weight_matrix_with_ids: |
  layer, current, change
  GBSTAC|chelsa-clim|bio1, 0.7, 0.6
  GBSTAC|chelsa-clim|bio2, 0.3, 0.4
data>pyLoadObservations>pyLoadObservations.yml@96|data_source: gbif_api
data>pyLoadObservations>pyLoadObservations.yml@96|max_year: 2020
data>pyLoadObservations>pyLoadObservations.yml@96|min_year: 2010
pipeline@121:
  - Acer saccharum
pipeline@128: 1000
pipeline@137:
  - chelsa-clim|bio1
  - chelsa-clim|bio3
  - chelsa-clim|bio4
  - chelsa-clim|bio12
  - chelsa-clim|bio15
pipeline@57: EPSG:6622
pipeline@58:
  - -2316297
  - -1971146
  - 1015207
  - 1511916

@gottacatchenall
Copy link
Contributor Author

gottacatchenall commented Sep 23, 2024

@jmlord This almost works, but we are having issues with permissions where we can't write to the /tmp dir, which ArchGDAL.jl has to do to save geotiffs

the issue with not being able to write tiffs because of permissions issues was this nested write call.

Everything works the same (afaict) not returning ArchGDAL.write but just using write! to burn into the raster band and returning, no idea why this specifically was a permissions problem

@tpoisot
Copy link

tpoisot commented Sep 23, 2024

the issue with not being able to write tiffs because of permissions issues was this nested write call.

Everything works the same (afaict) not returning ArchGDAL.write but just using write! to burn into the raster band and returning, no idea why this specifically was a permissions problem

🤷🏻 ArchGDAL, true to the Arch part of its name!

Can you open a PR with a fix?

@gottacatchenall
Copy link
Contributor Author

@jmlord
Copy link
Contributor

jmlord commented Sep 24, 2024

Is this a leftover that we can now remove, or was it intended to be plugged somewhere?
image

@gottacatchenall
Copy link
Contributor Author

yeah that isn't used

@jmlord
Copy link
Contributor

jmlord commented Sep 24, 2024

Great, could you please remove it and document it more thoroughly so that we can send this to an external reviewer?

ex:
tuning curve --> What does it mean, guidelines for interpretation?
range map thresholded at todo --> seems unfinished

A clear and concise description would be necessary for all IO. Remember that we now support markdown in descriptions if you feel it needs more.

Once you update the script yml, just remove and add back the output boxes and save it to update the pipeline json file with the new desc.

FYI I ran it and it went fine here.

@jmlord
Copy link
Contributor

jmlord commented Oct 3, 2024

When this works I can also add a quick bootstrap uncertainty - @jmlord / @JoryGriffith what do you think?

I think that could go in another PR, since we were thinking to officially open a review process tomorrow:

  • opening the review process Friday 4th (tomorrow), closing it Friday 11th.
  • publishing the pull request on Discourse to make it more transparent
  • sending specific invitations for reviewer (Jamie, Laura, etc.)

@gottacatchenall
Copy link
Contributor Author

Rebased and it looks like it works now, just need to clean up the documentation a bit

@tpoisot
Copy link

tpoisot commented Oct 9, 2024

We're likely going to be hitting JuliaGeo/GDAL.jl#179 at some point - so if the runs fails in the next days, that's why

@jmlord
Copy link
Contributor

jmlord commented Oct 9, 2024

We're likely going to be hitting JuliaGeo/GDAL.jl#179 at some point - so if the runs fails in the next days, that's why

Can we freeze the dependency version?

@tpoisot
Copy link

tpoisot commented Oct 9, 2024

yeah, we can also temporarily add LERC_jll@3and it seems to work - but maybe 1.10 isn't affected, and I have good hope that the GDAL_jll maintainers will actually solve this since it's affecting everyone

@gottacatchenall
Copy link
Contributor Author

Added docs here and here

@glaroc
Copy link
Contributor

glaroc commented Oct 15, 2024

@gottacatchenall Can you please change the output fit statistics to be a json file, as opposed to a json object inside the output.json? We haven't yet implemented viewing json objects directly in the output.json file.

@gottacatchenall
Copy link
Contributor Author

@gottacatchenall Can you please change the output fit statistics to be a json file, as opposed to a json object inside the output.json? We haven't yet implemented viewing json objects directly in the output.json file.

@glaroc Are you fully up to date? It already does afaict.

open(output_json_path, "w") do f

@glaroc glaroc self-requested a review October 15, 2024 21:35
Copy link
Contributor

@glaroc glaroc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it and all looks fine on my side.

@jmlord
Copy link
Contributor

jmlord commented Oct 16, 2024

@jamiemkass This is the new SDM pipeline I was mentioning.

@tpoisot
Copy link

tpoisot commented Oct 17, 2024

👋🏻 Hi @jamiemkass — this is a first working-but-rough draft of the pipeline, so we're keen to hear your thoughts!

@glaroc glaroc merged commit 3242524 into main Oct 17, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants