add option to disable IERS downloads #427

jchiang87 · 2023-10-06T21:25:45Z

No description provided.

cwwalter · 2023-10-08T19:53:15Z

I don't think I see the file:: URL to tell it where to get the local file in this case. Does it go somewhere else, or not checked in?

jchiang87 · 2023-10-08T19:56:42Z

Doing

iers.conf.auto_download = False
iers.conf.iers_degraded_accuracy = 'warn'

disables the download entirely, so there doesn't need to be any URLs specified and we don't need to keep local copies of the IERS corrections and leap seconds files around.

jchiang87 · 2023-10-08T19:58:15Z

Unfortunately, I just found out for the multiprocessing runs, we need to have these lines called for each process, in addition to the main process. Adding the lines to batoid_wcs.py seems to work.

cwwalter · 2023-10-08T19:58:55Z

Ah OK. For my understanding: Do having the file around do anything different at all if the calculation is for the future?

cwwalter

LGTM

jchiang87 · 2023-10-08T20:07:19Z

Do having the file around do anything different at all if the calculation is for the future?

According to the docs, the IERS download data are predictive 1 year into the future. So if we are computing for a date less than a year into the future, the most recent file would be provide numbers within the IERS accuracy spec.

rmjarvis

Seems fine to me. The only thought I had was maybe the default should be True. I can't imagine we ever need to be maximally up to date on IERS leap seconds.

jchiang87 · 2023-10-09T22:42:11Z

@jmeyers314 I added some code to batoid_wcs.py to disable the IERS downloads and also to silence those erfa warnings. Could you have a look please? Also, I see that batoid_rubin wants to download a bend.yaml file to its data/bend folder. This is causing my tests to fail when I run using a shared stack at S3DF. Those downloads may also be a problem for running on Theta in the batch queues, where remote downloads seemingly aren't allowed, but I guess they happen only in certain circumstances?

jmeyers314 · 2023-10-09T22:59:23Z

Looks good to me.

We can manually do the batoid_rubin data download somewhere appropriate instead of letting it try to auto-download.

jchiang87 · 2023-11-01T16:05:44Z

Unless I hear otherwise, I'll merge this at 2pm PT today.

rmjarvis · 2023-11-01T17:50:53Z

imsim/opsim_data.py

@@ -365,6 +374,10 @@ def OpsimData(config, base, value_type):
    """
    meta = galsim.config.GetInputObj('opsim_data', config, base, 'OpsimData')

+    if 'disable_iers_downloads' not in galsim.config.eval_base_variables:
+        galsim.config.eval_base_variables.append('disable_iers_downloads')
+        base['disable_iers_downloads'] = meta.disable_iers_downloads


I don't think this is the way we want to do this. You're effectively making our default template (which uses "$disable_iers_downloads") implicitly dependent on OpsimData. If a user doesn't include that anywhere, then the default template file will be broken in a very hard to diagnose way.

You should either have the template explicitly include this in eval_variables and have both batoid_wcs and opsim_data use that as the source information, or have one of them use a "Current" item to get it from the other one, so at least the user can see the way they are dependent on each other. E.g.

wcs: type: Batoid disable_iers_downloads: "@input.opsim_data.disable_iers_downloads"

If the latter, then you should add a note that if the user isn't using either of our two forms of opsim_data (skycat or instcat) then this should be set by hand in the wcs section.

Also, do we really need it in both places? It seems like the implementation is such that if either one has it, then it will get implemented and the astropy global variables will be set correctly. So maybe the better solution is just to identify which one needs it to be set correctly first. (I would expect opsim_data, but I might be wrong.) Then just do it there and don't bother with having it also done in the other place.

I'll also reiterate my suggestion to make the default True. I don't think we probably have any use cases where we want to guarantee that these are fully up to date, so letting it always be disabled seems like the better option to me.

The problem is that each process needs to have the download disabled, so if output.nproc > 1, the distributed processes, i.e., the ones running batoid for each CCD, would still try to do the download even if it's already been disabled for opsim_data in the head process.

OK. Is there a need to allow disable_iers_downloads=False? Why not just hard-code this entirely and always make it disabled when using imsim?

Making disable_iers_downloads=True the default seems fine, and I don't personally have a problem with hard-coding it here, but maybe someone would want it enabled in some cases? I really couldn't say. @jmeyers314 @cwwalter Do you have an opinion?

I can't imagine why I'd ever care about the precise iers numbers. This is basically degenerate with a pointing error when aiming the telescope, but probably much smaller.

cwwalter · 2023-11-01T21:04:44Z

I can imagine we want to do something where we recreate real observations after operations start.

But, as long as the errors are much smaller than the pointing accuracy this should be find. It would probably be good to have a sense of how big an effect this was.

rmjarvis · 2023-11-01T21:37:58Z

I think there is zero chance that any use case will require the iers updates. Even trying to reproduce actual observations. We don't know the actual telescope pointing well enough for this to matter.
But even if I'm wrong, it's trivial for a user to get the update if they really want it.

python -c "from astropy.time import Time; Time.now().tai"

will download the necessary data if the current data is out of date. If you want, we can provide that as a standalone script people can use to get the update if they want it.

…created by calling .__init__

…ccess it

…sim-config template

… fluxes from diffsky galaxies

jmeyers314

LGTM.

jchiang87 · 2023-11-17T20:42:08Z

Thanks everyone for helping on this one!

cwwalter approved these changes Oct 8, 2023

View reviewed changes

rmjarvis approved these changes Oct 8, 2023

View reviewed changes

jchiang87 requested a review from jmeyers314 October 9, 2023 22:38

jchiang87 force-pushed the u/jchiang/iers_download_handling branch from a3a6ea5 to 470f24d Compare November 1, 2023 03:36

rmjarvis reviewed Nov 1, 2023

View reviewed changes

jchiang87 added 14 commits November 2, 2023 07:32

add option to disable IERS downloads

732faf3

set .disable_iers_downloads for OpsimDataLoader objects that are not …

f31ba0d

…created by calling .__init__

add function decorator to suppress erfa warnings

2125598

silence erfa warnings

b36a834

make disable_iers_downloads an eval variable so that batoid_wcs can a…

781b099

…ccess it

add utility function to disable the iers downloads

3469c4d

change object type log message to info level

1ce47f5

add default value of disable_iers_downloads to template config

4b7a911

set eval_variables.bdisable_iers_downloads in test code instead of im…

1b636f2

…sim-config template

add disable_iers_downloads to test_stamp_szie config

355029d

print galsim version in test output

b77f5f4

summarily disable iers downloads

e6b3fe2

ignore iers warnings

7372dda

add test to confirm iers downloads are disabled

30c5138

jchiang87 force-pushed the u/jchiang/iers_download_handling branch from 9a98c82 to 30c5138 Compare November 2, 2023 14:32

jchiang87 and others added 4 commits November 12, 2023 15:33

add Mike's hot-fix for the skyCatalogs BaseObject._bp500 bandpass

ab42b34

ignore erfa warnings in ccd.py and readout.py; guard against negative…

c95b9e6

… fluxes from diffsky galaxies

Bump CI

18949ff

Adjust a couple tests whose fluxes changed with the bp500 fix

c731a61

jmeyers314 approved these changes Nov 17, 2023

View reviewed changes

jchiang87 marked this pull request as ready for review November 17, 2023 20:41

jchiang87 merged commit 84cfb30 into main Nov 17, 2023
3 checks passed

jchiang87 deleted the u/jchiang/iers_download_handling branch November 17, 2023 20:42

jchiang87 mentioned this pull request May 9, 2024

Allow local IERS file redux #426

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add option to disable IERS downloads #427

add option to disable IERS downloads #427

jchiang87 commented Oct 6, 2023

cwwalter commented Oct 8, 2023

jchiang87 commented Oct 8, 2023

jchiang87 commented Oct 8, 2023

cwwalter commented Oct 8, 2023

cwwalter left a comment

jchiang87 commented Oct 8, 2023

rmjarvis left a comment

jchiang87 commented Oct 9, 2023

jmeyers314 commented Oct 9, 2023

jchiang87 commented Nov 1, 2023

rmjarvis Nov 1, 2023

rmjarvis Nov 1, 2023

jchiang87 Nov 1, 2023

rmjarvis Nov 1, 2023

jchiang87 Nov 1, 2023 •

edited

Loading

jmeyers314 Nov 1, 2023

cwwalter commented Nov 1, 2023

rmjarvis commented Nov 1, 2023

jmeyers314 left a comment

jchiang87 commented Nov 17, 2023

add option to disable IERS downloads #427

add option to disable IERS downloads #427

Conversation

jchiang87 commented Oct 6, 2023

cwwalter commented Oct 8, 2023

jchiang87 commented Oct 8, 2023

jchiang87 commented Oct 8, 2023

cwwalter commented Oct 8, 2023

cwwalter left a comment

Choose a reason for hiding this comment

jchiang87 commented Oct 8, 2023

rmjarvis left a comment

Choose a reason for hiding this comment

jchiang87 commented Oct 9, 2023

jmeyers314 commented Oct 9, 2023

jchiang87 commented Nov 1, 2023

rmjarvis Nov 1, 2023

Choose a reason for hiding this comment

rmjarvis Nov 1, 2023

Choose a reason for hiding this comment

jchiang87 Nov 1, 2023

Choose a reason for hiding this comment

rmjarvis Nov 1, 2023

Choose a reason for hiding this comment

jchiang87 Nov 1, 2023 • edited Loading

Choose a reason for hiding this comment

jmeyers314 Nov 1, 2023

Choose a reason for hiding this comment

cwwalter commented Nov 1, 2023

rmjarvis commented Nov 1, 2023

jmeyers314 left a comment

Choose a reason for hiding this comment

jchiang87 commented Nov 17, 2023

jchiang87 Nov 1, 2023 •

edited

Loading