Addition of DESI EDR to SPARCL

[jacquesalice]

Assigning @adambolton to review. Note that this notebook in its current state won't run until the switchover from STAGE to PROD. To test the notebook in the meantime, make the following changes:

1. !pip install sparclclient==1.2.0b4
2. client = SparclClient(url='https://sparclstage.datalab.noirlab.edu)
3. qc.set_profile('db01')
4. In the Data Lab query, change FROM sparcl.main to FROM sparcl.main_new

[adambolton]

Great notebook! It runs successfully for me in its entirety (after the indicated tweaks). For the record, I ran it on my Mac laptop at home using a VPN connection. Everything looks good except for one issue and one maybe-issue.

Issue: I don't think reorder() is ready for prime time in the case of results from retrieve_by_specid(), since specid is not unique within DESI-EDR (or between datasets for that matter), and res.reorder() has unexpected/ill-defined behavior when it encounters multiple instances of a single specid. I happened to trip this behavior in my own testing because my random set of 20 records became a set of 21 records via retrieve_by_specid(), but then dropped back down to 20 after reorder(). (The particular specid with multiple records was 39627752092991638 if you want to reproduce.)

I would recommend that we either (a) remove the demonstration of reorder() for the case of results from retrieve_by_specid(), (b) add a warning that using reorder() with results from retrieve_by_specid() may cause unanticipated behavior (this is what is done in the user guide right now), or (c) demonstrate some sort of work-around (which is probably not a good idea for right now given our timeline.) We also probably want a ticket for improving this in the future.

Maybe-issue: the call to client.missing() took much longer to complete than I remember, but maybe that is as expected since the database is bigger now: CPU times: user 18.7 ms, sys: 4.29 ms, total: 23 ms Wall time: 43.6 s

One other thing: I don't know if I have permission to merge this PR b/c I don't know if I have write permission on this notebooks-latest repo.

[jacquesalice]

Thanks @adambolton , I went ahead and removed the reorder example using specids. I assigned @rnikutta to approve this PR

[rnikutta]

Hi @jacquesalice Trying to review this NB (running locally on my laptop, being on VPN, and following the pip install and profile setting instructions you posted). However, at this line I'm getting an error:

print(sorted(client.get_all_fields(dataset_list=['DESI-EDR'])))

KeyError: 'DESI-EDR'

I'm sure I'm doing something wrong...

[adambolton]

Are you running on STAGE? Because if so, it doesn't have DESI-EDR anymore as of the STAGE/PROD swap this morning.

On Mon, Jun 12, 2023 at 10:06 AM Robert Nikutta @.***> wrote:

Hi @jacquesalice https://github.com/jacquesalice Trying to review this NB (running locally on my laptop, being on VPN, and following the pip install and profile setting instructions you posted). However, at this line I'm getting an error:

print(sorted(client.get_all_fields(dataset_list=['DESI-EDR'])))

KeyError: 'DESI-EDR'

I'm sure I'm doing something wrong...

— Reply to this email directly, view it on GitHub https://github.com/astro-datalab/notebooks-latest/pull/174#issuecomment-1587729164, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEKENRCG3TXZI6ENVN5JMI3XK5ECNANCNFSM6AAAAAAY7V7Y3M . You are receiving this because you were mentioned.Message ID: @.***>

-- Adam S. Bolton Director, Community Science and Data Center NSF’s National Optical-Infrared Astronomy Research Laboratory

NEW EMAIL: @.*** [bolton-at-noao-dot-edu has been deactivated]

[rnikutta]

Ah that was it! I hadn't realized that the swap had already occurred. Thanks @adambolton . Testing on...

BTW, if I find no issues, do you want me to merge, or wait until tomorrow? (merge does immediately make the NB visible to all DL users, and on Github).

[jacquesalice]

Thanks @adambolton! And @rnikutta , I believe it would be ok to merge the NB today as long as we don't announce it anywhere until tomorrow.

[rnikutta]

@jacquesalice NB runs good on my local laptop, if I execute the pip install cell. On gp12, without the pip install, it fails when instantiating the client:

client = SparclClient()

---------------------------------------------------------------------------
Exception                                 Traceback (most recent call last)
<ipython-input-5-90a5240d1631> in <module>
----> 1 client = SparclClient()
      2 client

/data0/sw/anaconda3/lib/python3.8/site-packages/sparcl/client.py in __init__(self, url, verbose, connect_timeout, read_timeout)
    165                    f'{self.apiversion} from the SPARCL Server '
    166                    f'at {self.apiurl}.')
--> 167             raise Exception(msg)
    168         #self.session = requests.Session() #@@@
    169 

Exception: The SPARCL Client you are running expects an older version of the API services. Please upgrade to the latest "sparclclient".  The Client you are using expected version 8.0 but got 9.0 from the SPARCL Server at https://astrosparcl.datalab.noirlab.edu/sparc.

Does the client need to be updated on gp12 maybe?

[jacquesalice]

@rnikutta oh thank you for reminding me yes the client does need to be updated on gp12. Should I ask Mike to do that or maybe Igor? Cause it looks like Mike is on vacation for the rest of the month.....

[rnikutta]

I'll ask Mike right away. He said he's working half-time the next two weeks.

[rnikutta]

Just update to latest, or a specific version?

[jacquesalice]

@rnikutta Update to latest (pip install sparclclient). It's changed from 1.1.0 to 1.2.0, but you don't need to specify the version

[stephjuneau]

Hi @rnikutta & @adambolton I'm seeing that even though you wrote some comments/review above, your reviews are still blocking the merging of this PR. Any more changes needed for this notebook? Otherwise, we can merge when we're happy with it (I'd like to mention it to the DESI data team in my status update before our morning telecon if that works out).

[jacquesalice]

Hi @stephjuneau I believe the delay is because the sparclclient hasn't been updated on gp12 yet (@mjfitzpatrick is the only one that can do this, apparently) so the notebook doesn't actually work unless the users uncomment the pip install cell and specify a release version. If we want to have this notebook available for tomorrow morning I can temporarily edit the pip install cell so that it does install the most recent sparclclient version and the notebook is merge-able.

[jacquesalice]

Ah @adambolton this PR isn't ready for merge yet!! See my previous comment to Stephanie

[adambolton]

Oops sorry!

On Mon, Jun 12, 2023 at 10:08 PM Alice Jacques @.***> wrote:

Ah @adambolton https://github.com/adambolton this PR isn't ready for merge yet!! See my previous comment to Stephanie

— Reply to this email directly, view it on GitHub https://github.com/astro-datalab/notebooks-latest/pull/174#issuecomment-1588554689, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEKENRHGX3YNSPWRBKR7TYTXK7YV5ANCNFSM6AAAAAAY7V7Y3M . You are receiving this because you were mentioned.Message ID: @.***>

-- Adam S. Bolton Director, Community Science and Data Center NSF’s National Optical-Infrared Astronomy Research Laboratory

NEW EMAIL: @.*** [bolton-at-noao-dot-edu has been deactivated]

[jacquesalice]

That's ok! I just made a PR to revert this PR if you can Approve it and merge it https://github.com/astro-datalab/notebooks-latest/pull/177

[rnikutta]

May I propose that reviewers don't merge? (unless they are Alice or myself hehe ;-)))

[stephjuneau]

Hi @jacquesalice I think Mike might have actually updated the kernels on gp12 but it wasn't widely announced. Or at least Igor went to try to make the updated but he suspected Mike had beat him to it. I just tested in my notebook account and it now works as expected: (sparclclient:1.2.0, api:9.0, https://astrosparcl.datalab.noirlab.edu/sparc, verbose=False, connect_timeout=1.1, read_timeout=5400.0). So your new notebook should be ready to test and merge.