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

Added extra stats to be sent with MrDLib recommendations #4452

Merged
merged 9 commits into from
Dec 28, 2018

Conversation

conorfos
Copy link
Contributor

@conorfos conorfos commented Nov 2, 2018

  • Added the following stats to hopefully improve quality of recommendations:
    ** OS, OS version, Java version, Timezone
  • Added a couple of minor fixes
    ** Changed endpoint url
    ** Updated name of some of the expected JSON fields

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Manually tested changed features in running JabRef
  • Screenshots added in PR description (for bigger UI changes)
  • Ensured that the git commit message is a good one
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)

@stefan-kolb
Copy link
Member

Hm, what do the properties OS, OS version, Java version, Timezone have to do with the recommendations? 🤔

@conorfos
Copy link
Contributor Author

conorfos commented Nov 5, 2018

Hi @stefan-kolb ,

First, let us provide some background information for those who are not familiar with the recommender system and the changes. In JabRef 4.x, the following data was sent to Mr. DLib: partner_id ("jabref"), app_id ("jabref_desktop") app_version (JabRef version), and app_lang (JabRef language). For JabRef 5.x we would like to add the Operating System and Version, Java Version and Timezone. Ideally, we would sooner or later add additional information such as screen resolution and the window size of JabRef.

Generally, the more information a recommender system has about its users, the better the recommendations can potentially be. We will use all collected information to apply machine learning, i.e. learning in which context which items or recommendation algorithms may be best. Which information may be needed can never be said in advance, only after we have done some experiments. For instance, in some situations, there are notable differences between Apple and Windows users when it comes to recommendations (see e.g. https://www.forbes.com/sites/adriankingsleyhughes/2012/06/26/mac-users-have-money-to-spare-says-orbitz/). Hence, the operating system is a valuable piece of information. We don't know if the same applies e.g. to the JAVA version, but we would like to try it out. Hence, we would like to transmit this information.

We know that there is always a trade-off between privacy and recommendation effectiveness. In this case, we feel that OS, OS version, Java version, and Timezone would not pose a significant risk to user privacy. We also made it clear in the "Mr. DLib for JabRef information page" http://mr-dlib.org/information-for-users/information-about-mr-dlib-for-jabref-users/ what data is being collected and why. We would also be happy to add this information to the initial consent dialogue in JabRef. (The web page mentions that we would collect some data such as screen resolution though we don't do this yet. We thought we would just mention it on the page so that users are aware of our medium-term plans.)

In the long run, we would like to have three options in JabRef relating to the recommender system. 1. Deactivate 2. Standard Recommendations (that would be recommendations as they are now) 3. Advanced Personalized Recommendations (with unique user IDs) for which users, again, would have to explicitly opt-in.

We are happy to discuss this further over Skype or any other method that suits.
Thanks.

@conorfos
Copy link
Contributor Author

conorfos commented Nov 13, 2018

I am adding a mockup here as a suggestion of changes we could make to the privacy dialog so that users could decide between different recommendation services. We would be happy to discuss further with the team if that would be helpful.
jabref-dialog-2 0

@LinusDietz
Copy link
Member

Hi @conorfos,
there have been ongoing discussions among the JabRef team and we have come to the following final conclusion for the current issue:

You may use the Title (of the article), OS, and Timezone for the recommendations.

For the 'basic recommendation' based on the title the current opt-in is sufficient. For the 'advanced recommendations' using (OS, and Timezone) there must be an explicit user opt-in for each of these.

This is our compromise trading off your efforts for recommender systems research and protecting our users' privacy, also accounting for what is reasonable with regard to the general principle of data economy.

@Joeran
Copy link

Joeran commented Nov 29, 2018

Hi @LinusDietz ,

ok, it's no problem to change the dialogue to make the users agree for every piece of data explicitly. Regarding the specific data to be transferred, I am not sure if I am understanding you correctly.

Currently, the "old" JabRef that uses Mr. DLib, transfers the JabRef version and I believe also the language. Now, you mention that we are allowed to submit OS and timezone. Is that in addition to or instead of the JabRef version and language? To us, the JabRef version and language are actually more important for the recommendations than OS and timezone . The JabRef version may also be important one day to ensure backwards compatibility with our service.

Also, do we understand you correctly that you do not want us to create a unique user id as part of the "advanced recommendations", in which case we would not be able to give personalized recommendations?

Best,
Joeran

@LinusDietz
Copy link
Member

Whoops, I overlooked that one. Of course, you can also use the JabRef Version and the Language as done before.

Regarding personalized recommendations, we have not reached any consensus/decision yet.

@conorfos
Copy link
Contributor Author

conorfos commented Dec 12, 2018

I have implemented this functionality as agreed. Below is a screenshot of the privacy dialog and the code sending data can be found in the MrDLibFetcher class.
Thanks!
jabref privacy dialog

Copy link
Member

@LinusDietz LinusDietz left a comment

Choose a reason for hiding this comment

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

From my perspective it looks good. Some variables names could be improved, and some methods simplified, but I'd suggest to merge this, since it took very long to incorporate this PR anyways. Thanks to everyone discussing this!

@LinusDietz LinusDietz merged commit 3a09fa2 into master Dec 28, 2018
@Siedlerchr Siedlerchr deleted the MrDLib_request_stats branch December 28, 2018 18:47
Siedlerchr added a commit that referenced this pull request Dec 29, 2018
* upstream/master:
  Refactor BibEntry deprecated method (#4554)
  Added extra stats to be sent with MrDLib recommendations (#4452)
Siedlerchr added a commit that referenced this pull request Jan 4, 2019
* upstream/master:
  Bump checkstyle from 8.15 to 8.16 (#4562)
  Bump xmpbox from 2.0.12 to 2.0.13 (#4561)
  Delete the deprecated BibEntry Constructor (#4560)
  Refactor BibEntry deprecated method (#4554)
  Added extra stats to be sent with MrDLib recommendations (#4452)
  improve styling of preferences side menu (#4556)
  Cleanup interfaces (#4553)
Siedlerchr added a commit that referenced this pull request Jan 10, 2019
* 'ooPanel' of github.com:JabRef/jabref:
  Refactor BibEntry deprecated method (#4554)
  Added extra stats to be sent with MrDLib recommendations (#4452)
  checkstyle
  change mac default settings to LO path
  Add Book as preview as well
  fix style file select layout and inline variable'
  improve styling of preferences side menu (#4556)
  Cleanup interfaces (#4553)
frasca80 pushed a commit to frasca80/jabref that referenced this pull request Jan 14, 2019
Siedlerchr added a commit that referenced this pull request Jan 20, 2019
* upstream/master: (583 commits)
  update jfoenix and gradle plugins Replace outdated transformer log4j2 with official new one
  Update journalList.txt
  Fix for Issue #4437 - Some bugs in preference->Entry table columns (#4546)
  Don't set column sort type at startup (#4577)
  Add uncaught exception message (#4565)
  Converts integrity check dialog to JavaFX (#4559)
  Do not extract file ending from Urls (#4547)
  Bump checkstyle from 8.15 to 8.16 (#4562)
  Bump xmpbox from 2.0.12 to 2.0.13 (#4561)
  Delete the deprecated BibEntry Constructor (#4560)
  Refactor BibEntry deprecated method (#4554)
  Added extra stats to be sent with MrDLib recommendations (#4452)
  improve styling of preferences side menu (#4556)
  Cleanup interfaces (#4553)
  Bump fontbox from 2.0.12 to 2.0.13 (#4552)
  Bump pdfbox from 2.0.12 to 2.0.13 (#4551)
  Bump wiremock from 2.19.0 to 2.20.0 (#4550)
  Fixes that renaming a group did not change the group name in the interface (#4549)
  Bump applicationinsights-logging-log4j2 from 2.2.1 to 2.3.0 (#4540)
  Bump antlr4-runtime from 4.7.1 to 4.7.2 (#4542)
  ...

# Conflicts:
#	src/main/java/org/jabref/gui/JabRefFrame.java
#	src/main/java/org/jabref/model/entry/BibtexString.java
Siedlerchr pushed a commit that referenced this pull request Jan 25, 2019
* Cleanup interfaces (#4553)

* improve styling of preferences side menu (#4556)

* Added extra stats to be sent with MrDLib recommendations (#4452)

* Refactor BibEntry deprecated method (#4554)

* Refactor BibEntry deprecated method

* Fixed error

* More on checkstyle fixing

* Fixed checkstyle issues

* Added custom entrytype for types not registered in the enumerator.

* Added getTypeOrDefault method refactor code to use it and fix NPE problem

* Fixing checkstyle rules

* More on checkstyle

* More on getType getTypeOrDefault replacement

* Revert Article EntryType into Electronic

* Added break line between different packages

* Refactor BibtextEntryTypes.getTypeOrDefault method

* Removed unused import

* Removed extra new line, checkstyle error fixing

* Delete the deprecated BibEntry Constructor (#4560)

* Bump xmpbox from 2.0.12 to 2.0.13 (#4561)

Bumps xmpbox from 2.0.12 to 2.0.13.

Signed-off-by: dependabot[bot] <[email protected]>

* Bump checkstyle from 8.15 to 8.16 (#4562)

Bumps [checkstyle](https://github.com/checkstyle/checkstyle) from 8.15 to 8.16.
- [Release notes](https://github.com/checkstyle/checkstyle/releases)
- [Commits](checkstyle/checkstyle@checkstyle-8.15...checkstyle-8.16)

Signed-off-by: dependabot[bot] <[email protected]>

* Do not extract file ending from Urls (#4547)

* Fixes #4544 Do not extract file ending from Urls

* Add tests

* file type for any resource type

* Keep simple file name extraction for files

* checkstyle

* Converts integrity check dialog to JavaFX (#4559)

* Converts integrity check dialog to JavaFX

Moreover:
- Show entry by reference and not by id. Fixes #2181.
- Fixes a few issues that occurred when opening the entry editor by code from the integrity dialog
- Reuse gridpane in entry editor (should have a slightly superior performance)
- Improve display of progress dialog
- Invoke copy files task using central task executor

* fix l10n
fix aborting of copy files task and showing of integrity check dialog

* fix l10n

* Add uncaught exception message (#4565)

* Add error message for uncaught exceptions

Added a new view to the project. It's shown after the uncaught exception is logged.

* Add simple text to fallback error view

Added a label asking the user to look into the logfiles for more details.

* Add error message to language files

Added the error message to the german and english language files.

* Add ErrorDialogAndWait

Removed the FallbackErrorView. Added the showErrorDialogAndWait call instead.

* BibTex parser triggered on focus out of the text area instead of on value change event

* BibTex parser triggered on focus out of the text area instead of on value change event

* Added a post-parsing validation to be sure there's no relevant unconsidered content into epilog.
Added DialogService in SourceTab as current NotificationPane seems not working.

* Added a post-parsing validation to be sure there's no relevant unconsidered content into epilog.
Added DialogService in SourceTab as current NotificationPane seems not working.

* Codacy/PR Quality Review change.

* Codacy/PR Quality Review change.
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.

5 participants