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

Changing the default method for download.CRUNCEP #2426

Merged
merged 9 commits into from
Sep 20, 2019

Conversation

para2x
Copy link
Contributor

@para2x para2x commented Sep 19, 2019

Description

Now that @ashiklom, @infotroph and I all confirmed (issue #2424 ) that the opendap method as a default breaks the CRUNCEP.download, falling back to ncss method seems to solve the problem for both me and @ashiklom .

Motivation and Context

Review Time Estimate

  • Immediately
  • Within one week
  • When possible

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the CHANGELOG.md.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@infotroph
Copy link
Member

@para2x why does this change config.example.php?

@para2x
Copy link
Contributor Author

para2x commented Sep 19, 2019

@infotroph just fixed it

Copy link
Member

@infotroph infotroph left a comment

Choose a reason for hiding this comment

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

Test passes:

0> git fetch para2x                           chrisb@morus:~/github_forks/pecan
remote: Enumerating objects: 28, done.
remote: Counting objects: 100% (28/28), done.
remote: Compressing objects: 100% (9/9), done.
remote: Total 19 (delta 15), reused 14 (delta 10), pack-reused 0
Unpacking objects: 100% (19/19), done.
From github.com:para2x/pecan
 * [new branch]          CRUNCEP_Adress_Update -> para2x/CRUNCEP_Adress_Update
0> git checkout CRUNCEP_Adress_Update         chrisb@morus:~/github_forks/pecan
Branch 'CRUNCEP_Adress_Update' set up to track remote branch 'CRUNCEP_Adress_Update' from 'para2x'.
Switched to a new branch 'CRUNCEP_Adress_Update'
0> Rscript -e 'devtools::test("modules/data.atmosphere", "CRUNCEP")
quote> '
Loading PEcAn.data.atmosphere
Loading required package: methods
Testing PEcAn.data.atmosphere
✔ |  OK F W S | Context
⠏ |   0       | Checking CRUNCEP downloadtrying URL 'https://thredds.daac.ornl.gov/thredds/ncss/ornldaac/1220/mstmip_driver_global_hd_landwatermask_v1.nc4?var=land_water_mask&disableLLSubset=on&disableProjSubset=on&horizStride=1&accept=netcdf'
downloaded 262 KB

trying URL 'https://thredds.daac.ornl.gov/thredds/ncss/grid/ornldaac/1220/mstmip_driver_global_hd_climate_tair_2000_v1.nc4/dataset.html?var=tair&south=40&west=-88&north=40.000005&east=-87.999995&time_start=2000-01-01T00:00:00Z&time_end=2000-12-31T21:00:00Z&accept=netcdf'
downloaded 24 KB

trying URL 'https://thredds.daac.ornl.gov/thredds/ncss/grid/ornldaac/1220/mstmip_driver_global_hd_climate_lwdown_2000_v1.nc4/dataset.html?var=lwdown&south=40&west=-88&north=40.000005&east=-87.999995&time_start=2000-01-01T00:00:00Z&time_end=2000-12-31T21:00:00Z&accept=netcdf'
downloaded 24 KB

trying URL 'https://thredds.daac.ornl.gov/thredds/ncss/grid/ornldaac/1220/mstmip_driver_global_hd_climate_press_2000_v1.nc4/dataset.html?var=press&south=40&west=-88&north=40.000005&east=-87.999995&time_start=2000-01-01T00:00:00Z&time_end=2000-12-31T21:00:00Z&accept=netcdf'
downloaded 24 KB

trying URL 'https://thredds.daac.ornl.gov/thredds/ncss/grid/ornldaac/1220/mstmip_driver_global_hd_climate_swdown_2000_v1.nc4/dataset.html?var=swdown&south=40&west=-88&north=40.000005&east=-87.999995&time_start=2000-01-01T00:00:00Z&time_end=2000-12-31T21:00:00Z&accept=netcdf'
downloaded 24 KB

trying URL 'https://thredds.daac.ornl.gov/thredds/ncss/grid/ornldaac/1220/mstmip_driver_global_hd_climate_uwind_2000_v1.nc4/dataset.html?var=uwind&south=40&west=-88&north=40.000005&east=-87.999995&time_start=2000-01-01T00:00:00Z&time_end=2000-12-31T21:00:00Z&accept=netcdf'
downloaded 24 KB

trying URL 'https://thredds.daac.ornl.gov/thredds/ncss/grid/ornldaac/1220/mstmip_driver_global_hd_climate_vwind_2000_v1.nc4/dataset.html?var=vwind&south=40&west=-88&north=40.000005&east=-87.999995&time_start=2000-01-01T00:00:00Z&time_end=2000-12-31T21:00:00Z&accept=netcdf'
downloaded 24 KB

trying URL 'https://thredds.daac.ornl.gov/thredds/ncss/grid/ornldaac/1220/mstmip_driver_global_hd_climate_qair_2000_v1.nc4/dataset.html?var=qair&south=40&west=-88&north=40.000005&east=-87.999995&time_start=2000-01-01T00:00:00Z&time_end=2000-12-31T21:00:00Z&accept=netcdf'
downloaded 24 KB

trying URL 'https://thredds.daac.ornl.gov/thredds/ncss/grid/ornldaac/1220/mstmip_driver_global_hd_climate_rain_2000_v1.nc4/dataset.html?var=rain&south=40&west=-88&north=40.000005&east=-87.999995&time_start=2000-01-01T00:00:00Z&time_end=2000-12-31T21:00:00Z&accept=netcdf'
downloaded 24 KB

✔ |   3       | Checking CRUNCEP download [92.5 s]

══ Results ═════════════════════════════════════════════════════════════════════
Duration: 92.8 s

OK:       3
Failed:   0
Warnings: 0
Skipped:  0
0>                                            chrisb@morus:~/github_forks/pecan

...But it also seems like we should probably get in touch with DAAC to find out what's up with the opendap server -- it's worrying for one method to fail without notice and it makes me wonder whether switching to ncss is setting us up for trouble later.

@ashiklom
Copy link
Member

ashiklom commented Sep 19, 2019

it makes me wonder whether switching to ncss is setting us up for trouble later

To me, the NetCDF subset (what I called "ncss") seems more reliable because it uses only a single REST-like query per download, rather than the more complicated back-and-forth that OpenDAP seems to do. OpenDAP is nicer for interactive use because it lets you explore the file as if it were local, but I wonder if it's less robust to the large, high-frequency throughput we often use it for.

@para2x para2x requested a review from mdietze September 19, 2019 21:00
Copy link
Member

@dlebauer dlebauer left a comment

Choose a reason for hiding this comment

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

updated changelog para2x@095db69

@mdietze mdietze merged commit d402489 into PecanProject:develop Sep 20, 2019
@infotroph infotroph mentioned this pull request Oct 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants