Fixing black column problem in kriging#15
Fixing black column problem in kriging#15rafaelalmeida wants to merge 4 commits intoTheHortonMachine:masterfrom
Conversation
- Changed strategy to pre-calculate grid positions, avoiding rounding errors; - Revived one of the unit tests for the kriging module; - Some documentation fixes and improvements; - Minor refactorings for cleaner code.
Changing NumericUtils class to use class-specific max() methods to avoid compilation errors in newer environments (shouldn't affect older ones)
|
I still could not get the build to pass in Travis. @moovida, when you can, could you give me some advice on how to reproduce the build locally? Eclipse says everything's OK. |
|
Hi @rafaelalmeida , sorry for not coming back to you earlier. First off, thanks for this contribution, I really hope to get this going. Locally the build will be tested just by running maven as: mvn install But I am quite sure you are already doing this, so I am not sure what you mean. |
|
Hi @moovida, no problem!
Even then it runs fine on my machine, but at least now I have a much better idea of where the error is. I'll try to get the checks passing before next week, so you can spend your time on a higher-level look at the solution. |
There was a problem hiding this comment.
did you remove the org.jgrasstools because of your custom build?
I assume this might be a problem when building. Didn't notice that before.
|
Nice changes @rafaelalmeida , nice to see you enabled the testcase for it. That is a huge plus. I built it on my machine and it went smooth also. The test takes some time, it would be great to have a smaller one. Do you happen to have one at hand? |
|
@moovida I believe I removed that line simply because Eclipse sent me a warning saying it was redundant. I can add it back, but I don't think that's the problem. Travis says I don't have a smaller testcase at hand, but I can build an artificial one with a few points. The current test case data is the one that was already present (albeit commented-out) in the repository. Is the artificial test case better than the current one? |
|
Hi @rafaelalmeida , sorry for my late late replies, life is currently quite hard on me with time for the open source projects. Just know that I really appreciate your contributions.
No, you are right, I didn't notice that it was double. And it did indeed build.
The test was probably commented out because it took more than the rest of the build and I don't think there was a real check behind it. I.e. the results looked good, so the author placed that as a testcase. But I agree it is not simple to create testcases for certain modules. |
|
Hi @moovida! I haven't forgotten this, but I'm also out of time for the next couple days. I expect to complete this update as soon as possible! |
|
Don't worry @rafaelalmeida , there is no hurry. Thanks for keeping this in mind. I am also very busy in this period, so at least we are aligned :-) |
2 participants
My attempt to solve #12.
@moovida, do you think this would work? I tested with the first test from the unit tests, and also with some of my own data.
I am afraid this could fail in swapped-axis projections, or projections where the y axis is not a northing (that is, it "grows" downwards, not upwards), but it seems to me that in these cases, the previously existing code for getCoordinate would fail anyway, if I understand correctly.