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

WIP: Add Windows CI and make compiler arguments platform dependent. #40

Closed
wants to merge 4 commits into from

Conversation

cpaulik
Copy link
Contributor

@cpaulik cpaulik commented Jun 30, 2016

You can see an example of how it runs and fails at https://ci.appveyor.com/project/cpaulik/pyresample/build/1.0.5

It fails because of the is_nan function which is called differently on windows.
In pytesmo I had the same problem in the past and worked around that by using the npy_isnan function like this:

cdef extern from "numpy/npy_math.h":
    bint npy_isnan(double x)

If you want to support this then we can refine this PR. For now I copied my build configuration with little modifications.

Fix #39 when finished.

@mraspaud
Copy link
Member

Wow, thanks a lot ! Yes, the isnan fix would be nice, definitely.

@djhoese
Copy link
Member

djhoese commented Jun 30, 2016

I'm working on changing the isnan stuff now. thanks.

@cpaulik
Copy link
Contributor Author

cpaulik commented Jun 30, 2016

@davidh-ssec Cool. Will you PR into my branch or should I rebase this PR onto your branch when you are finished?

@djhoese
Copy link
Member

djhoese commented Jun 30, 2016

Well we'll want to make a new PR so it points to the pre-master branch instead of master. I'll either do that as a new PR or just a simple merge to pre-master and then we'll just close this and reference that commit. Right now I'm having trouble figuring out how to get appveyor to build my new branch. Not seeing an obvious way of doing it in the UI.

@cpaulik
Copy link
Contributor Author

cpaulik commented Jun 30, 2016

Ok, then I'll close this and you merge it as you see fit.

Appveyor should pick it up as soon as it is activated for the project. At least that what happened in my case. But you might have to push a new commit for it to actually start building.

@cpaulik cpaulik closed this Jun 30, 2016
@cpaulik
Copy link
Contributor Author

cpaulik commented Jun 30, 2016

Actually that was too soon. I've seen that basemap is not available for windows and python > 2 on the regular conda channels. So you'll probably want this last commit also.

@cpaulik cpaulik reopened this Jun 30, 2016
@djhoese
Copy link
Member

djhoese commented Jun 30, 2016

Thanks.

@cpaulik cpaulik closed this Jun 30, 2016
@djhoese
Copy link
Member

djhoese commented Jun 30, 2016

I think I've fixed all of the C errors I was getting but now it fails when trying to run the unittests. @cpaulik do you have any ideas:

https://ci.appveyor.com/project/davidh-ssec/pyresample-0waq3/build/job/x781hjwigx2lrwtm

My current code is in fix-windows-ewa. I have a couple meetings to go to now, but will try to get back to this this afternoon.

@djhoese djhoese reopened this Jun 30, 2016
@djhoese djhoese self-assigned this Jun 30, 2016
@djhoese
Copy link
Member

djhoese commented Jul 1, 2016

The tests never actually seem to finish. Later today I'll try to boot up a Windows VM and see if I can actually run the pyresample tests and figure this out.

@djhoese
Copy link
Member

djhoese commented Jul 5, 2016

@cpaulik I'm trying to compile with python 3.5 and keep getting these warnings and errors. Do you have any idea how I can fix the "fpclassify" error? I'm trying to figure out why the tests that are failing are failing.

pyresample/ewa/_fornav_templates.cpp(253): warning C4244: '=': conversion from 'npy_float64' to 'weight_type', possible
loss of data
pyresample/ewa/_fornav_templates.cpp(259): warning C4244: '=': conversion from 'npy_float64' to 'weight_type', possible
loss of data
pyresample/ewa/_fornav_templates.cpp(395): warning C4305: '=': truncation from 'double' to 'weight_type'
pyresample/ewa/_fornav_templates.cpp(439): note: see reference to function template instantiation 'unsigned int write_gr
id_image<npy_float32>(npy_float32 *,npy_float32,std::size_t,std::size_t,accum_type *,weight_type *,int,weight_type)' bei
ng compiled
C:\Program Files (x86)\Windows Kits\10\include\10.0.10586.0\ucrt\math.h(416): error C2668: 'fpclassify': ambiguous call
to overloaded function
C:\Program Files (x86)\Windows Kits\10\include\10.0.10586.0\ucrt\math.h(301): note: could be 'int fpclassify(long double
) throw()'
C:\Program Files (x86)\Windows Kits\10\include\10.0.10586.0\ucrt\math.h(296): note: or       'int fpclassify(double) thr
ow()'
C:\Program Files (x86)\Windows Kits\10\include\10.0.10586.0\ucrt\math.h(291): note: or       'int fpclassify(float) thro
w()'
C:\Program Files (x86)\Windows Kits\10\include\10.0.10586.0\ucrt\math.h(416): note: while trying to match the argument l
ist '(npy_int8)'
pyresample/ewa/_fornav_templates.cpp(276): note: see reference to function template instantiation 'bool isnan<IMAGE_TYPE
>(_Ty) throw()' being compiled
        with
        [
            IMAGE_TYPE=npy_int8,
            _Ty=npy_int8
        ]
pyresample/ewa/_fornav_templates.cpp(430): note: see reference to function template instantiation 'int compute_ewa<npy_f
loat32,npy_int8>(std::size_t,int,std::size_t,std::size_t,std::size_t,std::size_t,npy_float32 *,npy_float32 *,npy_int8 **
,npy_int8,accum_type **,weight_type **,ewa_weight *,ewa_parameters *)' being compiled
error: command 'C:\\Program Files (x86)\\Microsoft Visual Studio 14.0\\VC\\BIN\\amd64\\cl.exe' failed with exit status 2

@cpaulik
Copy link
Contributor Author

cpaulik commented Jul 6, 2016

@davidh-ssec It seems that I've opened a can of worms here 😄 . Unfortunately I don't know much about C++ so I can't help.

@djhoese
Copy link
Member

djhoese commented Jul 6, 2016

No problem @cpaulik. I know that it builds on appveyor so now I just need to make it work on my Windows VM so I can figure out why certain tests are failing. I just need to start with the right compiler and make sure I have all the assets required.

@djhoese
Copy link
Member

djhoese commented Jul 12, 2016

Just a small progress note, I'm on vacation but had some time/interest to look at this and I think my specific problem with building on my Windows VM is related to a new C++ compiler and the isnan function possibly being called even for integer inputs.

@djhoese
Copy link
Member

djhoese commented Jul 13, 2016

Wow, just realized the build problem I'm having with python 3.5 on my Windows VM is also experienced on appveyor. For these specific test problems I'll make sure to test with python 3.4...assuming I can get it working on my VM.

@djhoese
Copy link
Member

djhoese commented Jul 14, 2016

After a day and a half of installing new frameworks, failing, uninstalling those frameworks, failing, and then installing some more frameworks I finally got the python 3.4 stuff to build on my Windows VM and I get the same test failures as appveyor. They seem to be tied to multiprocessing which is always fun to debug.

The simple proof is pyresample.test.test_geometry.Test.test_grid_filter2D which uses nprocs=2, when I set it to 1 it passes. Let's see how much sleep I want to get tonight.

@djhoese
Copy link
Member

djhoese commented Jul 14, 2016

The issue with test_grid_filter2D is in the Proj_MP class. I only saw failures when running python setup.py test, but running nosetest they ran fine. The problem turned out to be windows multiprocessing rerunning the original command. Since the setup.py didn't include an if __name__ == "__main__" the tests were being rerun for every test using multiprocessing.

https://bugs.python.org/issue11240

This should be fixed now on my branch, but the test_nearest_resize still seems to run indefinitely.

@djhoese
Copy link
Member

djhoese commented Jul 14, 2016

@mraspaud Any idea why the warnings checks are limited to certain versions of python:

        if (sys.version_info < (2, 6) or
                (sys.version_info >= (3, 0) and sys.version_info < (3, 4))):
            swath_def = geometry.BaseDefinition(lons1, lats1)
        else:
            with warnings.catch_warnings(record=True) as w1:
                swath_def = geometry.BaseDefinition(lons1, lats1)
                self.assertFalse(
                    len(w1) != 1, 'Failed to trigger a warning on longitude wrapping')
                self.assertFalse(('-180:+180' not in str(w1[0].message)),
                                 'Failed to trigger correct warning about longitude wrapping')

@mraspaud
Copy link
Member

Probably because it was failing in the other versions. As mentioned in the
other bug report, I suspect we have to check the warnings more
thoroughly...

On Thu, 14 Jul 2016, 13:38 David Hoese, notifications@github.com wrote:

@mraspaud https://github.com/mraspaud Any idea why the warnings checks
are limited to certain versions of python:

    if (sys.version_info < (2, 6) or
            (sys.version_info >= (3, 0) and sys.version_info < (3, 4))):
        swath_def = geometry.BaseDefinition(lons1, lats1)
    else:
        with warnings.catch_warnings(record=True) as w1:
            swath_def = geometry.BaseDefinition(lons1, lats1)
            self.assertFalse(
                len(w1) != 1, 'Failed to trigger a warning on longitude wrapping')
            self.assertFalse(('-180:+180' not in str(w1[0].message)),
                             'Failed to trigger correct warning about longitude wrapping')


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#40 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAKPeo6tcvm1wq50aYqcLCAT1XKrD_b9ks5qVh-tgaJpZM4JCKLf
.

Martin Raspaud, PhD
Software engineer
SMHI, SE-60176

@djhoese
Copy link
Member

djhoese commented Jul 15, 2016

@cpaulik I've got everything working except with python 3.4 there are warnings that aren't generated (or more that are), but I only see them on appveyor and not on my own python 3.4 environment. Do you have any ideas what might be going on?

@cpaulik
Copy link
Contributor Author

cpaulik commented Jul 18, 2016

@davidh-ssec No not really. But it seems that you got it working now? At least the last appveyor build shows all green.

@djhoese
Copy link
Member

djhoese commented Jul 18, 2016

Yes I'm trying out a new branch that borrowed from some stuff the astropy package does. It looks like when checking if warnings were raised it is best to clear out all previous warnings otherwise your tests depend on what order they were run in. I'm hoping to clean it up and merge this all back in to pyresample pre-master for people to test. I'm also waiting on a merge for pykdtree that should make it work on Windows which would make the pyresample tests 5 times faster.

@cpaulik
Copy link
Contributor Author

cpaulik commented Jul 18, 2016

That's great news. Thanks a lot for taking the time to make this work.

David Hoese writes:

Yes I'm trying out a new branch that borrowed from some stuff the astropy package does. It looks like when checking if warnings were raised it is best to clear out all previous warnings otherwise your tests depend on what order they were run in. I'm hoping to clean it up and merge this all back in to pyresample pre-master for people to test. I'm also waiting on a merge for pykdtree that should make it work on Windows which would make the pyresample tests 5 times faster.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#40 (comment)


Christoph Paulik
Twitter, Github: @cpaulik
PGP: 8CFC D7DF 2867 B2DC 749B 1B0A 6E3B A262 5186 A0AC

@djhoese
Copy link
Member

djhoese commented Jul 18, 2016

@cpaulik If possible could you try the pre-master branch on your windows environments? Everything passes on appveyor it just takes ~25-35 minutes per environment since we don't have pykdtree released for Windows yet. See storpipfugl/pykdtree#11 for details on that.

Feel free to ask questions or report bugs for this specific feature on this PR, but if you have code suggestions feel free to make a PR pointing to pre-master or a new feature branch. But since this seems to be fixed I'm going to close this PR.

Thanks a lot for your help getting appveyor set up.

@coveralls
Copy link

Coverage Status

Coverage remained the same at ?% when pulling affcc82 on cpaulik:add-windows-CI into 4462759 on pytroll:master.

pnuu pushed a commit to pnuu/pyresample that referenced this pull request Nov 29, 2018
…version

Feature check atmcorr lut version
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.

4 participants