Opened 14 months ago

Closed 9 months ago

#1188 closed task (fixed)

Make WCPS 1.5 parser production ready

Reported by: vmerticariu Owned by: bphamhuu
Priority: major Milestone: 9.3
Component: petascope Version: development
Keywords: Cc: mdumitru
Complexity: Medium

Description

The WCPS 2.0 parser can be accessed via the ProcessCoverages? extension, using the wcpsVersion=2.0 parameter, e.g.:

http://localhost.com:8080/rasdaman/ows?service=WCS&version=2.0.1&request=ProcessCoverages&query=for c in ( AvgLandTemp? ) return encode(c[Lat(53.08), Long(8.80), ansi("2014-07")], "csv")&wcpsVersion=2.0

The task is to make the parser production ready:

  • create a branch in patchmanager called wcps2_parser (email @dmisev)
  • on the new branch, switch the wcps systemtests to use the new version (i.e. change the wcps requests URL)
  • run the systemtests and fix the parser where the tests don't pass (please leave coverageConstructor, generalCondeser and switch for the end)
  • when all current tests pass, add more systemtests in case you find operations that are missing

Change History (29)

comment:1 Changed 14 months ago by vmerticariu

  • Component changed from undecided to petascope

comment:2 Changed 14 months ago by dmisev

  • Milestone set to 10.0

comment:3 Changed 14 months ago by dmisev

I created a branch feature_wcps2, Bang please submit patches for this ticket on that branch.

comment:4 Changed 14 months ago by bphamhuu

thanks Dimitar for creating branch and Vlad for your assignment (with detail information), I will do it now.

comment:5 Changed 14 months ago by mdumitru

Bang, please focus on this ticket first, it's crucial that we get this right for the next release.

comment:6 Changed 14 months ago by bphamhuu

Yes, Alex, I has focused on it, I don't understand this requirement, as I think WCPS 2.0 will have more operations than WCPS 1.0? Maybe systemtest in WCPS 1.0 is not cover enought. Could you explain as how can I know about WCPS 2.0 description to test it correctly (?), e.g grammar, operations,...

when all current tests pass, add more systemtests in case you find operations that are missing

comment:7 Changed 14 months ago by mdumitru

Hey, This is not really WCPS 2.0 but more like WCPS 1.5. The language is exactly the same, the parser and the handling was changed though.
All the code for this is in the wcps2 packaged of petascope.
All existing WCPS queries including the ones in the systemtest should work the same with this version of the parser. If they don't it should be considered a bug that needs to be fixed.

First you should make the systemtest run on the url Vlad indicated and report all the failing cases.

comment:8 Changed 14 months ago by bphamhuu

Right, I just think about this case and the above I think should not have problem. Thanks.

comment:9 Changed 14 months ago by bphamhuu

Hey, I've looked up the first request which add &wcpsVersion=2.0 behind the query, the problem is now in systemtest, it is configured to test wcps
with

WCPS_URL="localhost:8080/rasdaman/ows/wcps"

I can change the link to use

​http://localhost.com:8080/rasdaman/ows?service=WCS&version=2.0.1&request=ProcessCoverages&query=...&wcpsVersion=2.0

but beside the WCPS queries in text, it also has lot of WCPS queries in XML like below, what should I change here?

<?xml version="1.0" encoding="UTF-8" ?>
<ProcessCoveragesRequest xmlns="http://www.opengis.net/wcps/1.0" service="WCPS" version="1.0.0">
<query><xmlSyntax><coverageIterator><iteratorVar>a</iteratorVar><coverageName>rgb</coverageName></coverageIterator><condense><opPlus/><iterator><iteratorVar>x</iteratorVar><axis>x</axis>
<numericConstant>1</numericConstant><numericConstant>10</numericConstant></iterator><mult><variableRef>x</variableRef>
<fieldSelect><slice><coverage>a</coverage><axis>i</axis><slicingPosition><variableRef>x</variableRef></slicingPosition><axis>j</axis>
<slicingPosition><numericConstant>10</numericConstant></slicingPosition></slice><field><name>red</name></field></fieldSelect></mult></condense></xmlSyntax></query></ProcessCoveragesRequest>

comment:10 Changed 14 months ago by dmisev

Bang, you can ignore these "xml" queries, they are not used in the systemtest. You can't even submit them in WCPS 1.5, the XML parser doesn't exist I believe.

comment:11 Changed 14 months ago by bphamhuu

thanks, Dimitar, it is easier now.

comment:12 Changed 14 months ago by bphamhuu

Thanks for comments, this is the report for 16 test cases fail with WCPS2 https://docs.google.com/spreadsheets/d/1vRTJ4If30tcHaPF5qsGusR4Mai2X0QHTmhswBK82ei8/edit?usp=sharing

What do you want in "fix the parser where the tests don't pass" ? I have to fix in systemtest or in Petascope. I think it is hard if have to fix in Petascope but nice to try.

comment:13 Changed 14 months ago by vmerticariu

There are probably some problems with the parser, for these cases you need to fix in petascope.

Some others are oracle problems, where you could fix in systemtests.

You need to understand what the query does, and which is the expected output, then you can decide where you need to apply the fix.

To get started with that, please make your report more complete:

  • I see that for the most cases you write "Difference File Size, same gdalinfo". Does this mean that the 2 queries return images that look the same, for example? What is the reason the file size is different?
  • What is the reason the test fails - byte comparison, one of the queries results in error or other?
  • If the error texts that are returned are different, what are the results?

I suggest you write in the column "version 2.0" the actual output of the query 2.0(e.g. image or error or somthing else). Same for 1.0. Add a column explaining why the test fails.

comment:14 Changed 14 months ago by bphamhuu

Hey Vlad, thanks for comments, I've added detail to the report. Now, you can see:
+ Difference between texts if it is long.
+ Difference between image errors (bytes, gdalinfo or the real output).

https://docs.google.com/spreadsheets/d/1vRTJ4If30tcHaPF5qsGusR4Mai2X0QHTmhswBK82ei8/edit#gid=0

Then you can tell me which cases I should fix it first (you have 14 failed cases in here).

comment:15 Changed 14 months ago by vmerticariu

Hi Bang, nicely done!

Next step would be to look at the query and write, in another column, which is the correct response (version 1 or version 2), and why.

comment:16 Changed 14 months ago by bphamhuu

@all: I've tested both WCPS versions and get the rasql query for you to compare (I don't know which query is true or false in the case of especially when ticket #1021 is appeared in here with few cases (pink color in comment column) has difference values due to origin.

Some of query with encode(...,"csv") returns more values or error in WCPS 2 rasql query (dark purple color in comment).

See the column Link Diff with these cases.

https://docs.google.com/spreadsheets/d/1vRTJ4If30tcHaPF5qsGusR4Mai2X0QHTmhswBK82ei8/edit#gid=0

Conclusion: WCPS 1.0 can compare WCPS 2.0 with the problems in origin (need discussion!), the others errors is due to WCPS 2.0 parser or encode(coverage, "csv).

comment:17 Changed 14 months ago by dmisev

One difference is pretty noticeable -- WCPS 2 doesn't set the xmin/xmax/.. in the encode function correctly corresponding to the subset, but always specifies the full extent of the coverage?

comment:18 Changed 14 months ago by bphamhuu

Vlad, In WCPS 2.0, allow support expression in interval? as I've tested and doing with error case "95-just_slice_asterisk.error.test" when WCPS 2.0 allow to parses (should be error case).

Below is another example which returns 2 different values.

+ This returns 36

http://localhost:8080/rasdaman/ows?service=WCS&version=2.0.1&request=ProcessCoverages
&query=for c in (mr) return max(c[j(0)])&wcpsVersion=2.0

+ This returns 15

http://localhost:8080/rasdaman/ows?service=WCS&version=2.0.1&request=ProcessCoverages
&query=for c in (mr) return max(c[j(0+0)])&wcpsVersion=2.0

I aksed this because in wcps.g4 (parser), I see this rule

dimensionPointElement: axisName (COLON crsName)? LEFT_PARANTHESIS coverageExpression RIGHT_PARANTHESIS    

and it is invalid query in WCPS 1.0

comment:19 Changed 14 months ago by dmisev

Bang, I suggest that for every issue you open a separate ticket, as it will get unwieldy to put all in a single ticket I guess.

comment:20 Changed 14 months ago by bphamhuu

Dimitar, yes, but I just want to ask to get information (e.g if it is invalid, a ticket will be opened after that).

comment:21 follow-up: Changed 14 months ago by dmisev

Ok so comment:18, what do you think the right behavior should be? Should WCPS 2.0 support coverage expressions or not? Obviously slicing with an asterisk is an error though so that needs to be fixed.

comment:22 in reply to: ↑ 21 Changed 14 months ago by bphamhuu

Replying to dmisev:

Ok so comment:18, what do you think the right behavior should be? Should WCPS 2.0 support coverage expressions or not? Obviously slicing with an asterisk is an error though so that needs to be fixed.

Dimitar, I'm asking with coverage expression e,g c[i(0+0)]. Of course slicing with an asterisk is an error (I don't ask in here) but I want to ask in WCPS 2.0, it support c[i(0+0)] also? But c[i(0+0)] is different with c[i(0)].

After that, I can open 2 tickets or just 1 ticket for problem with asterisk.

Last edited 14 months ago by bphamhuu (previous) (diff)

comment:23 Changed 14 months ago by dmisev

Yeah c[i(0)] obviously should be equal to c[i(0+0)], I'm not quite sure why would it give different result, so that's also a bug.

comment:24 Changed 14 months ago by bphamhuu

Ok, I've opened 10 tickets for problems as send to all of you also, then try to solve before next release, thanks.

comment:25 Changed 14 months ago by bphamhuu

Due to I've missed to change the error check also from test WCPS2.0 then it only returns 14 error cases, but when I change it correctly to use the URL for WCPS2.0 in error cases, it will returns 44 error and I will open new tickets as update with the current report also.

https://docs.google.com/spreadsheets/d/1vRTJ4If30tcHaPF5qsGusR4Mai2X0QHTmhswBK82ei8/edit#gid=0

if [[ $WGET_EXIT_CODE != 0 ]]; then
       echo "Error when processing WCPS request in KVP, query return error: "$WGET_EXIT_CODE
       wget_error "$WCPS_EMBEDDED_URL_1_5""$QUERY" "$out"  # USE THIS LINK
       #wget_error "$WCPS_EMBEDDED_URL""$QUERY" "$out" 
       echo ".Done"
fi

comment:26 Changed 14 months ago by mdumitru

Bang, maybe it would be better to open and fix one ticket at a time, I am assuming many of them are related so fixing one might fix 5 other tests.

comment:27 Changed 14 months ago by bphamhuu

Alex, I think it is best when know all errors then can group it later and choose which ticket to fix (thanks for comment).

comment:28 Changed 14 months ago by dmisev

I already suggested the same to Bang, it's enough tickets.

comment:29 Changed 9 months ago by bphamhuu

  • Milestone changed from 10.0 to 9.3
  • Resolution set to fixed
  • Status changed from new to closed
  • Summary changed from Make WCPS 2.0 parser production ready to Make WCPS 1.5 parser production ready

The patch of parsers and handlers for WCPS 1.5 is accepted, thanks for all your supports, I will close this ticket here.

Note: See TracTickets for help on using tickets.