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

Scoring problems #178

Open
dl1jbe opened this issue Jul 27, 2020 · 17 comments
Open

Scoring problems #178

dl1jbe opened this issue Jul 27, 2020 · 17 comments
Assignees
Labels

Comments

@dl1jbe
Copy link
Member

dl1jbe commented Jul 27, 2020

SP3RXO reported some problems with the scoring logic.

CONTINENTLIST=EU,AS
COUNTINENT_LIST_POINTS=2
USE_CONTINENT_LIST_ONLY
COUNTRY_MULT

gives 2 points for AS stations but -1 (!!!) for EU stations.

@dl1jbe dl1jbe self-assigned this Jul 27, 2020
@dl1jbe
Copy link
Member Author

dl1jbe commented Jul 27, 2020

Furthermore if USE_CONTINENT_LIST_ONLY gets dropped CONTINENT_LIST_POINTS will not be applied at all.

dl1jbe added a commit to dl1jbe/tlf that referenced this issue Nov 10, 2020
@dl1jbe dl1jbe mentioned this issue Nov 10, 2020
@dl1jbe
Copy link
Member Author

dl1jbe commented Nov 19, 2020

Part 1 of the problem got fixed by PR #189.

While thinking about the second part it showed that there is no easy solution. There are two main reasons for that:

Problem

  1. It is not really clear (and not easy to see) which role a CONTINENT_LIST_POINTS should play and which priority it has if other keywords (MYCOUNTRY_POINT, MYCONTINENT_POINT , ...) are set.

  2. The scoring mechanism has grown into a complicated peace of code. It started by points for my country, my continent and other continents. Due to new contests with more complex rules we added such things as COUNTY/CONTINENT_LIST and more. Besides not really defined priorities there are further problems as the continent and country lists are sometimes used for scoring points and sometimes for counting multis.

Possible solution

I would like to suggest a complete new mechanism which (hopefully) will avoid these problems and can led to a much more clean scoring code.

  1. There will be two list for point scoring of a) continents and b) countries

  2. In each list you can name a continent or country together with its points (e.g. EU 2 points, all other continents 3 point, DL in the other list with 1 point would give the known 1 point for own cty, 2 for own continent and 3 points for DX) . Different points for each continent/country are allowed.

  3. Continents/countries not in the lists will get no points at all.

  4. Country points will override continent points

Scoring will be simple:

a) Assume 0 points
b) If in Continentlist use the points from there
c) If in Countrylist override points so far by the points from the list

I suggest two new keywords:

  • COUNTRYPOINTS=<cty1,cty2,cty3:points1>;<cty4,....,cty_n:points2>; ...
  • CONTINENTPOINTS=<cnt1,cnt1:points1>;cnt3,...,cnt_n:points2>

More than one of each keywords are allowed in the config file. They are cumulative so you can split a long list of countries to more than one line.

Discussion

With the above changes we should be able to

a) drop CONTINENT/COUNTRYLIST_POINTS entirely from keywords (as we do not need them anymore) and ignore USE_COUNTRY/CONTINENTLIST_ONLY for scoring (but keep functionality for counting of multis).

b) change the other keywords (ONE-POINT, ..., MYCOUNTRY_POINT, ..., DX_POINTS) to use the above mechanism internally.

As far as I can see, these scheme should solve most of the cases. If not, please drop a note.

As that would deprecate some keywords I think the suggested change needs at least a new minor number (maybe tlf 1.5).

Any comments are welcome.

@dl1jbe
Copy link
Member Author

dl1jbe commented Nov 21, 2020

See https://github.com/dl1jbe/tlf/tree/new_scoring for a proof of concept implementation.
Please use it for experiments and report any problems (or success).

@dl1jbe dl1jbe added the BUG label Dec 30, 2020
@zcsahok
Copy link
Member

zcsahok commented Jan 28, 2021

Partly related: add_callarea() seems to be unused along with manise80 and callareas[].

@dl1jbe
Copy link
Member Author

dl1jbe commented Jan 28, 2021

Looks like preparations for some EA contest. But was already dead in 0.9.10 - the oldest version I could found. The complete addarea.[ch] should be dropped.

@zcsahok
Copy link
Member

zcsahok commented Feb 15, 2021

Another point for scoring:
when reading the log file the score field is used as-is.

        if (iscontest) {
            // get points
            total = total + log_get_points(inputbuffer);

I think that part of the log is not an input field in the strict sense: it was written based on some (potentially different) rules. Logically loading should kind of replay the inputs (~band/time/call/exchange) and update score/mults accordingly. Something like the way readcabrillo is doing it. Not too nice with all the "magic" global variables, but we don't have better at the moment.

At least scoring/mults got managable with the introduction of contest defs.

@dl1jbe
Copy link
Member Author

dl1jbe commented Feb 16, 2021

I think that part of the log is not an input field in the strict sense: it was written based on some (potentially different) rules. Logically loading should kind of replay the inputs (~band/time/call/exchange) and update score/mults accordingly. Something like the way readcabrillo is doing it. Not too nice with all the "magic" global variables, but we don't have better at the moment.

I think the origin of the above code lies in the way contest results where handled in the 80ies and 90ies. You had to hand in a written paper with your multis marked and your claimed points written. So in case TLF got points or multis wrong you could just leave TLF, correct the file and reread it in. Afterwards TLF used whatever you told.

In newer times the contest organizers are scoring multis and points by themself, so that feature really is no longer useful. I will open an issue and assign it to me.

@zcsahok
Copy link
Member

zcsahok commented Apr 5, 2023

Is there anything open for this issue?

@dl1jbe
Copy link
Member Author

dl1jbe commented Apr 7, 2023

The problem of USE_CONTINENTLIST_ONLY (see second comment) is still unsolved. Also open is a discussion of the suggested change of country and continent scoring from 20 Nov 21.

@zcsahok
Copy link
Member

zcsahok commented Apr 10, 2023

I see. USE_COUNTRYLIST_ONLY has also some "magic" quirks as it sets the value of countrylist_only flag based on which side of multi one is. Provided, of course, that this information is correct, i.e. the config keywords are in right order.

MY_xxx configs could be still helpful, I think. Evaluation shall go from specific to generic, so first check country, then continent.
How about something like this:

int points = -1;  //unset
if (countrynr == my.countrynr) {
   ... try MY_COUNTRY_POINTS, MY_CONTINENT_POINTS -- as the current code
}
if (points >=  0) {return points;} // got a match

if (inCountryList) {   // this could be also using the more generic country list + point config
  ... try points from the country list (or COUNTRY_LIST_POINTS)
}
if (points >=  0) {return points;} // got a match

if (from same continent) {
   ... try MY_CONTINTENT_POINTS
} 
if (points >=  0) {return points;} // got a match

if (check continent list) {
   ... try continentlist_points
}
if (points >=  0) {return points;} // got a match

.. apply DX_POINTS

return (points >= 0 ? points : 0);

In addition current xxx_LIST_POINTS has provisions for reading from a file and filtering based on contest name. That would be lost by using the new parsing. Not sure if that's a problem, just noted it.

Country lists have typically 1 or 2 entries and there is normally one list per contest. So not sure if the change of format is worth it. (even if it's more generic, for sure)

@zcsahok
Copy link
Member

zcsahok commented Apr 10, 2023

A good test vehicle would be ARRL-DX.

@dl1jbe
Copy link
Member Author

dl1jbe commented Apr 12, 2023

I see. USE_COUNTRYLIST_ONLY has also some "magic" quirks as it sets the value of countrylist_only flag based on which side of multi one is. Provided, of course, that this information is correct, i.e. the config keywords are in right order.

Yes, unluckily we also have no clear distinction between keywords for point scoring and multiplier handling. Some of the keywords have side effects for adding the QSO at all (see handling of non W/VE stations for some ARRL contests). We need to sort that out.

MY_xxx configs could be still helpful, I think. Evaluation shall go from specific to generic, so first check country, then continent. How about something like this:

int points = -1;  //unset
if (countrynr == my.countrynr) {
   ... try MY_COUNTRY_POINTS, MY_CONTINENT_POINTS -- as the current code
}
if (points >=  0) {return points;} // got a match

if (inCountryList) {   // this could be also using the more generic country list + point config
  ... try points from the country list (or COUNTRY_LIST_POINTS)
}
if (points >=  0) {return points;} // got a match

if (from same continent) {
   ... try MY_CONTINTENT_POINTS
} 
if (points >=  0) {return points;} // got a match

if (check continent list) {
   ... try continentlist_points
}
if (points >=  0) {return points;} // got a match

.. apply DX_POINTS

return (points >= 0 ? points : 0);

It is basically the same as current code (see default handling on scoreContinentOrCountry() in score.c) but handles also the problem with CONTINENTLIST and CONTINENTLIST_POINTS set if USE_CONTINENTLIST_ONLY not set. And that is missing for the original problem of this issue. I will prepare a PR for it.

In addition current xxx_LIST_POINTS has provisions for reading from a file and filtering based on contest name. That would be lost by using the new parsing. Not sure if that's a problem, just noted it.

That is true. But I am not sure if it is used widely.

Country lists have typically 1 or 2 entries and there is normally one list per contest. So not sure if the change of format is worth it. (even if it's more generic, for sure)

The idea of my suggested changes were more longterm - to try out new generalized rules for point and mulit scoring (which avoid most of the special case handling we have now) as an optional experiment and later on switch to a clean new sets of rules with a major version change of TLF.

@dl1jbe
Copy link
Member Author

dl1jbe commented Apr 12, 2023

It is basically the same as current code (see default handling on scoreContinentOrCountry() in score.c) but handles also the problem with CONTINENTLIST and CONTINENTLIST_POINTS set if USE_CONTINENTLIST_ONLY not set. And that is missing for the original problem of this issue. I will prepare a PR for it.

I have to correct myself. Original code returns 0 if actual country is my.country and neither country_points nor continent_points are set. Your code continues the checks for the other point sources (similar for the other ones).

dl1jbe added a commit to dl1jbe/tlf that referenced this issue Apr 12, 2023
@dl1jbe
Copy link
Member Author

dl1jbe commented Apr 12, 2023

See dl1jbe@b2702d1 for a possible implementation.

One note:

  • We are changing the actual implement scoring rules, especially the order of precedence for CONTRY_POINTS and CONTINENT_POINTS over COUNTRYLIST_POINTS. Let us hope that nobody uses these combination.

@zcsahok
Copy link
Member

zcsahok commented Apr 12, 2023

Looks good. Even if we change the scoring logic it becomes more logical and easier to follow/explain, which is a good thing.

The *ONLY keywords can be then dropped. Well, apart of the use of continentlist_only in addcall.c. The juggling with veto I find pretty obscure.

@dl1jbe
Copy link
Member Author

dl1jbe commented Apr 14, 2023

Ok, that means we should drop the initial cases handling the 'c/c_only' cases also. Same results can be reached with leaving the other point settings undefined.

The 'veto' handling came with the patch that introduced the country/continentlist and looks like a quick hack. Similar problems comes from the double use of the lists to control exclude multipliers (EXCLUDE_MULTILIST). I think it is urgent to sort that out and find a clean solution.

@dl1jbe
Copy link
Member Author

dl1jbe commented Apr 15, 2023

See PR #390

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants