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

Updates for osx compat #302

Merged
merged 5 commits into from
Jun 11, 2019
Merged

Updates for osx compat #302

merged 5 commits into from
Jun 11, 2019

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Mar 19, 2019

Updates for osx compat

Closes #263

@jeanconn jeanconn requested a review from taldcroft March 19, 2019 12:57
@jeanconn
Copy link
Contributor Author

I'd have liked to toss in the use of maude to make this more osx-mobile-ready, but the ACA model code is set to fetch 30 days of AACCCDPT, so for operational use I don't think we want to swap that over to use the full resolution data from the 5min stat it is doing, and it looks like we don't have stat support in fetch just yet.

@taldcroft
Copy link
Member

taldcroft commented Apr 1, 2019

AACCCDPT is sampled at 32.8 sec, so a MAUDE or CXC fetch of 30 days full resolution is no problem. About 80000 samples, which I checked is done in a couple of seconds. So go for it!

@jeanconn
Copy link
Contributor Author

jeanconn commented Jun 3, 2019

A MAUDE fetch of 30 days full resolution doesn't immediately seem to be option either:

Exception: MAUDE query failed: cannot query more than 7 days with allow_subset=False at line 28

starcheck/calc_ccd_temps.py Outdated Show resolved Hide resolved
starcheck/calc_ccd_temps.py Outdated Show resolved Hide resolved
starcheck/calc_ccd_temps.py Outdated Show resolved Hide resolved
Copy link
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

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

At some level I'd be happier with a more minimal change that just addresses OSX-specific problems, instead of also bundling "don't have an eng-archive" problems. I don't see a problem requiring that at least AACCCDPT is available to run starcheck. (Cutting pitch is fine). Since maude will likely never have some of the ACIS MSIDs, a general model of OSX support probably needs to include a requirement to have the right eng archive data available. That does not seem problematic to me.

starcheck/calc_ccd_temps.py Outdated Show resolved Hide resolved
starcheck/calc_ccd_temps.py Outdated Show resolved Hide resolved
@jeanconn
Copy link
Contributor Author

jeanconn commented Jun 6, 2019

OK. I read your "So go for it!" feedback as sort-of direction to go this way, but can back it out.

I also didn't see the starcheck code as a real general example? and thought there might be some benefit to the maude strategy as being both more up-to-date and a great fallback plan.

@jeanconn jeanconn requested a review from taldcroft June 10, 2019 14:44
@taldcroft
Copy link
Member

What do I need to do for testing? I did:

This gave me:

(ska3) monkeys$ ./sandbox_starcheck -dir ~/ska/data/mpcrit1/mplogs/2019/JUN1019 -out test
Fatal Python error: Py_Initialize: unable to load the file system codec
ModuleNotFoundError: No module named 'encodings'

Current thread 0x00007fff9841b380 (most recent call first):
Failed to autogenerate /private/var/folders/k1/2ktyqf0x2n7_fzvdnvljq2240000gq/T/starcheck_inline.XXXXXX.wUsWJXGx/config-darwin-2level-5.026002.

 at starcheck/src/lib/Ska/Starcheck/Obsid.pm line 162.
BEGIN failed--compilation aborted at starcheck/src/lib/Ska/Starcheck/Obsid.pm line 162.
Compilation failed in require at ./starcheck/src/starcheck.pl line 33.
BEGIN failed--compilation aborted at ./starcheck/src/starcheck.pl line 33.

@jeanconn
Copy link
Contributor Author

jeanconn commented Jun 10, 2019 via email

@taldcroft
Copy link
Member

OK, success, wow!!

I encountered some difficulties because sourcing ska_envs redefines $SKA to a new location which is not my SKA. So I made a data soft link but then made the mistake of running ska_sync with the new SKA, which blew away the soft link and made a second fresh data repo. So I would say this is a little fiddly right now for standalone Ska, but it does work!

Copy link
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

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

It works for me, meaning it ran to completion on JUN1019A and gave reasonable looking results. Did not do any regression test comparison, however.

@jeanconn
Copy link
Contributor Author

Maybe open a skare3 issue on the problem you had with ska_envs redefining SKA? I thought I had figured out the non-root environment scenarios but maybe missed something.

Starcheck might, in fact, work with just SKA set (without setting LD_LIBRARY_PATH and such from ska_envs.sh) but that wasn't how I was testing.

With regard to regression, I can't see how the changes that ended up in this PR could introduce regressions in flight starcheck on linux. Do you also want to certify osx for load review? I wasn't sure what we needed to test or if "it looks like it works" might be just fine for now.

@taldcroft
Copy link
Member

If I use export PYTHONHOME=$CONDA_PREFIX instead of $SKA then both starcheck/src/starcheck and sandbox_starcheck work in a standalone env.

@taldcroft
Copy link
Member

taldcroft commented Jun 11, 2019

About regressions, I was not implying that the linux output could change, I was wondering if the OSX output matches the linux output. That is likely the case, but does need checking for at least one load (by diffing the starcheck.txt output).

I don't foresee certifying use on OSX for flight load review right away, but I will certainly use starcheck on my mac and it would be a problem if it were giving results inconsistent with linux.

@jeanconn
Copy link
Contributor Author

OK, so was the regression comment an aside, do you want to see regression validation for osx in this PR, or should it be a new issue?

@jeanconn
Copy link
Contributor Author

jeanconn commented Jun 11, 2019

I think the real problem is that I replaced SKA_ARCH_OS with SKA in these scripts, but that we've decided that SKA only defines the path of data. For interaction with the Perl I think I do need something that always points to the Python installation (equivalent to old SKA_ROOT?), and that's a little counter to:

sot/skare3#22 (comment)

I think CONDA_PREFIX only works in an env, so would you want to do something like try it first? My thought was that we just want ska_envs.sh to try to do the right thing if possible and perhaps set SKA and SKA_ROOT to reasonable things in the env.

@jeanconn jeanconn merged commit 6eb234f into master Jun 11, 2019
@jeanconn jeanconn deleted the osx branch June 11, 2019 18:47
@taldcroft
Copy link
Member

For the record, I confirmed that the report .txt file on my Mac for JUN1019A is exactly the same as the output from linux except for the header info up top (including the sybase error on OSX). In both cases I ran ./starcheck_sandbox from the git repo.

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.

Doesn't quite work on osx
2 participants