R: Re: R: Re: kde review kartesio
Sure, I thought it was already back to playground-edu. Luca Tringali Messaggio originale Da: aa...@kde.org Data: 27/10/2013 19.03 A: LucaTringalitringalinv...@libero.it, kde-core-devel@kde.org Ogg: Re: R: Re: kde review kartesio El Diumenge, 27 d'octubre de 2013, a les 18:48:21, LucaTringali va escriure: Hi Albert, I'm working on the points but, since I'm kinda busy with my work, the code I have written for now is still not stable. I think it'll be ready in the first week of January 2014. If you study something, at high school or university, which includes experiments (chemistry, phisycs, biology, engineering, etc...) you need a best fit calculator. So, philosophically, it is something very similar to Kalzium Calculator. Ok, I see your point. Maybe we should move it back from kdereview to playgound-edu? It's not the idea that stuff stays in kdereview for a long time. Cheers, Albert Luca Tringali Messaggio originale Da: aa...@kde.org Data: 27/10/2013 18.30 A: kde-core-devel@kde.org Ogg: Re: kde review kartesio El Dijous, 9 de maig de 2013, a les 18:06:16, LucaTringali va escriure: Hello,I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval.I followed the guidelines (http://techbase.kde.org/Policies/Application_Lifecycle) and Kartesio is actually in KDE review:https://projects.kde.org/projects/kdereview/kartesioFor any question, ask me. Luca Tringali Hi Luca, have you worked on addressing the points raised in the comments you were made? Also I am wondering if kdeedu is really the place for this app, I understand it's a very useful application for science/laboratory situations, but is that really something a student would use? Cheers, Albert
R: Re: kde review kartesio
Hi Albert, I'm working on the points but, since I'm kinda busy with my work, the code I have written for now is still not stable. I think it'll be ready in the first week of January 2014. If you study something, at high school or university, which includes experiments (chemistry, phisycs, biology, engineering, etc...) you need a best fit calculator. So, philosophically, it is something very similar to Kalzium Calculator. Luca Tringali Messaggio originale Da: aa...@kde.org Data: 27/10/2013 18.30 A: kde-core-devel@kde.org Ogg: Re: kde review kartesio El Dijous, 9 de maig de 2013, a les 18:06:16, LucaTringali va escriure: Hello,I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval.I followed the guidelines (http://techbase.kde.org/Policies/Application_Lifecycle) and Kartesio is actually in KDE review:https://projects.kde.org/projects/kdereview/kartesioFor any question, ask me. Luca Tringali Hi Luca, have you worked on addressing the points raised in the comments you were made? Also I am wondering if kdeedu is really the place for this app, I understand it's a very useful application for science/laboratory situations, but is that really something a student would use? Cheers, Albert
R: Re: R: Re: kde review kartesio
Hi, no, build.sh is not needed. Since I'm kinda lazy, I prepared a shell script to run the following commands: mkdir build cd build sudo make uninstall make clean rm CMakeCache.txt cmake .. -DCMAKE_INSTALL_PREFIX=`kde4-config --prefix` make sudo make install so I don't have to type all those every time I want to build Kartesio again. I'm not sure why you get that error, mainlybecause I don't know which is the instruction that gives that problem since I changed a lot the code in these hours. Try to download the latest git version and build it, so I will know exactly where the problem is. Luca Tringali Messaggio originale Da: annemarie.mahf...@free.fr Data: 11/05/2013 12.04 A: LucaTringalitringalinv...@libero.it Cc: kde-core-devel@kde.org Ogg: Re: R: Re: kde review kartesio Hi, Hi, actually I have not prepared any binary package. Anyway, you can install Kartesio downloading the source code from the git repo (https://projects.kde. org/projects/kdereview/kartesio), installing the library libzorbaneural (https: //www.gitorious.org/zorbaneural/zorbaneural/trees/master/binary- packages/libzorbaneural-0.1), and running the build.sh script you find in the Kartesio root folder. Why is this build.sh script needed? Is it because I did not run it that I got my build error? Also, you should have installed the program maxima (just the program, dev libraries are not needed) to have Kartesio fully working. If there are some troubles in building Kartesio, just ask me. I have this error: /home/kde-devel/kartesio/src/calculations.cpp:278:1: error: control reaches end of non-void function [-Werror=return-type] cc1plus: some warnings being treated as errors Best regards, Anne-Marie
R: Re: kde review kartesio
Yes, I also think adding new features now is not a good idea in this moment.
This is the reason why the new version of Kartesio I uploaded to git (about 45
minutes ago) contains basically all the corrctions you suggested, but no new
features.
Luca Tringali
Messaggio originale
Da: annemarie.mahf...@free.fr
Data: 11/05/2013 12.15
A: LucaTringalitringalinv...@libero.it
Cc: kde-core-devel@kde.org
Ogg: Re: kde review kartesio
Hi,
In general, you're mixing a lot of plain C / stdlib stuff into Qt
code. Is there a reason for that? For example, in
calculations.cpp:148
you take text from a text field, convert it to a byte array, convert
it to a char* and then pass it to a function. Why not just pass the
QString? You can iterate over a QString like
foreach ( const QChar c, myqstring ) { ... }
or also
for ( int i = 0; i myqstring.size(); i++ ) { ... }
if you like that better, and you can also index it like a char*, as
in
mystring[i+1] or so.
Yes, this is an heritage from the older version of Kartesio, that was
based
mainly on plain ANSI C++. Those mixing are just an hack to make
Kartesio work
immediately. If I'll have time, I will translate everything into
Qt, but
first of all I would like to apply other features.
You asked for an inclusion in KDE and we are reviewing Kartesio. There is
already a big amount of work to be done from the comments you got. I don't
think adding features now is a smart move, review is a phase where your program
should reach KDE standards. Using Qt libs wherever possible is the priority and
getting all the required fixes will make you busy enough.
Best regards,
Anne-Marie
R: Re: R: [kartesio] src: Fix some strings
Hi, there is a little misunderstanding: those two settings are not global settings. This is how it works: if you check BackProp, the neural network will be trained using back propagation. If you check GenAlg, the network will be trained with genetic algorithms. If you check both them, the network will be trained using both methods. In some cases, you may want to use BackProp, in others GenAlg or both them: it depends on the points you are using and on the curve you want to fit. For example, I see that if you use a line as fittign function (y=m*x+q) BackProp is all you need. But if you want to use something more complex (like a third grade function, x^3) GenAlg gives you better results. So those settings are not global: the user must be able to change them every time he/she needs, because usually to find the best fitting curve you have to try different settings. Talking about the translation, BackProp and GenAlg, in my opinion, shouldn't be translated: they are commonly used in many countries (in Italy we use them too) by anybody usually works with neural networks. It would be like translating Okay or UFO: those are English espressions that are common in a lot of countries. Luca Tringali Messaggio originale Da: annemarie.mahf...@free.fr Data: 11/05/2013 18.02 A: LucaTringalitringalinv...@libero.it Ogg: Re: R: [kartesio] src: Fix some strings Hi, I know nothing about calculating best fit curves so maybe I am totally out of context. But the question that comes to my mind is: are there some settings that are always use in the same way like BackProp and GenAlg and that could be put in Kartesio global settings? I understand BackProp is used in English but what will we write in French or Ukrainian? Either those settings are moved elsewhere or they need translatable English strings with context info if necessary and tooltips to precise their meaning as well. Anne-Marie - Mail original - De: LucaTringali tringalinv...@libero.it À: an...@kde.org, kde-comm...@kde.org Envoyé: Samedi 11 Mai 2013 16:37:44 Objet: R: [kartesio] src: Fix some strings Hi, I don't know how to make the Iterations: label more near to the spinbox: I tried to set the text aligned to the right, but almost nothing changes. Talking about BackProp and GenAlg, they are the common abbreviation for Back propagation algorithm and Genetic algorithm. Those two texts look too long for me, so I decided to use the short version. Luca Tringali Messaggio originale Da: an...@kde.org Data: 11/05/2013 14.37 A: kde-comm...@kde.org Cc: tringalinv...@libero.it Ogg: [kartesio] src: Fix some strings Git commit 3c57b222dbc6496e362ce15e47b6c7d348cb3ca8 by Anne-Marie Mahfouf. Committed on 11/05/2013 at 14:33. Pushed by annma into branch 'master'. Fix some strings You should not have labels like BackProp and GenAlg on the interface Luca, please change them in some plain English ones. Also the widgets are not well aligned on the interface, for example the Iterations: label is separated from its spinbox. CCMAIL: tringalinv...@libero.it M +7-7src/mainwindow.ui http://commits.kde.org/kartesio/3c57b222dbc6496e362ce15e47b6c7d348cb3ca8 diff --git a/src/mainwindow.ui b/src/mainwindow.ui index c8a8fe6..8243520 100644 --- a/src/mainwindow.ui +++ b/src/mainwindow.ui @@ -16,7 +16,7 @@ item widget class=QLabel name=label_2 property name=text -stringHere's the best fit curve:/string +stringBest fit curve:/string /property /widget /item @@ -46,7 +46,7 @@ item widget class=QLabel name=label property name=text -stringWrite here a function like quot;y=a*(x^2)+b*x+cquot;: /string +stringWrite a function like quot;y=a*(x^2)+b*x+cquot;: /string /property /widget /item @@ -345,7 +345,7 @@ item widget class=QLabel name=label_3 property name=text -stringXmin/string +stringX min:/string /property /widget /item @@ -362,7 +362,7 @@ item widget class=QLabel name=label_4 property name=text -stringXmax/string +stringX max:/string /property /widget /item @@ -382,7 +382,7 @@ item widget class=QLabel name=label_5 property name=text -stringYmin/string +stringY min:/string /property /widget /item @@ -399,7 +399,7 @@ item widget class=QLabel name=label_6 property name=text -stringYmax/string +stringY max:/string /property /widget /item @@ -423,7 +423,7 @@ item widget class=QLabel name=label_8
Re: Re: kde review kartesio
Yes, Kartesio is used to calculate fitting curves for experimental points: I, as a chemistry student, already used it for some laboratory reports. Actually, there is no other program like this in KDE: obiously you could obtain something similar with RKward, but this one is too much complex for fitting curves, and students usually do not like R. Kartesio does one thing, and does it simply and good. Caculating best fit curves with R is smilar to cross a stream with the Queen Mary.The most interesting feature of Kartesio is that it allows you to write manually the equation you want to use to fit the points (with other programs like LibreOffice Calc it's possible only to use 3 or 4 already implemented and generic functions). For example, I can choose to fit my points with y=a*sin(b*x) or with y=(9.342/x)+c. Luca Tringali Messaggio originale Da: tcanabr...@kde.org Data: 10/05/2013 12.28 A: Anne-Marie Mahfoufannemarie.mahf...@free.fr Cc: LucaTringalitringalinv...@libero.it, kde-core-devel@kde.org Ogg: Re: kde review kartesio Quite Unlikely ... It's a Solver, to fit curves into points, That's very used in any theorical research, engeniering, math, phisics, etc. 2013/5/10 Anne-Marie Mahfouf annemarie.mahf...@free.fr Hi, I am wondering what is the user base for this application as it seems quite specialized (I did not build it yet though). Can you tell us more about the potential target? Another question that comes to mind is: can't it be a feature of an existing KDE Edu apps? Best regards, Anne-Marie - Mail original - De: LucaTringali tringalinv...@libero.it À: kde-core-devel@kde.org Envoyé: Jeudi 9 Mai 2013 18:06:16 Objet: kde review kartesio Hello, I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval. I followed the guidelines ( http://techbase.kde.org/Policies/Application_Lifecycle ) and Kartesio is actually in KDE review: https://projects.kde.org/projects/kdereview/kartesio For any question, ask me. Luca Tringali
R: Re: kde review kartesio
Hello, libzorbaneural can be found here: https://www.gitorious.org/zorbaneural/zorbaneural/trees/master Here are also some packages for the stable version (deb and rpm): https://www.gitorious.org/zorbaneural/zorbaneural/trees/master/binary- packages/libzorbaneural-0.1 Once you have installed it, the build should go fine. The screenshot folder and the .pro file are not needed, if they are a problem I can remove them. Talking about the system call in calculations, this is the reason why actuallt Kartesio works only on GNU/Linux systems. Why I did it? Because it was the easier way to do that. In the next release of Kartesio, this problem will be solved. I'm not very practical with translatable strings, so I excuse for the Message. sh: is there a wiki page to understand how to write a Message.sh file? I'm writing comments on variables in header files, in the next hours I'll publish them into git. Luca Tringali Messaggio originale Da: annemarie.mahf...@free.fr Data: 10/05/2013 13.58 A: LucaTringalitringalinv...@libero.it Cc: kde-core-devel@kde.org Ogg: Re: kde review kartesio Hi, A few primary remarks: - libzorbaneural is needed but my distro does not have anything with neural in it (OpenSuse 12.3) what repo do I need to add in order to get it? The libzorbaneural website should be added to the cmake file so people can find this and packagers can add it to their distros. - I see a screenshot folder and some .pro files that probably are not needed - some doxygen comments for the variables in the .h files would be appreciated, if anyone else wants to fix bugs it'll help a lot. - Kartesio does not build for me, I get /home/kde- devel/kartesio/src/calculations.cpp:278:1: error: control reaches end of non-void function [-Werror=return-type] cc1plus: some warnings being treated as errors - I don't see a Messages.sh file to extract translatable strings. - I am not comfortable with the rm call line 181 in calculations.cpp = you can probably use more Qt classes here and in other parts of this file too. That's only a quick review as I couldn't run the app yet. Tomaz, as for the user base maybe we could start a module for advanced scientific tools? Best regards, Anne-Marie - Mail original - De: Tomaz Canabrava tcanabr...@kde.org À: Anne-Marie Mahfouf annemarie.mahf...@free.fr Cc: LucaTringali tringalinv...@libero.it, kde-core-devel@kde.org Envoyé: Vendredi 10 Mai 2013 12:28:54 Objet: Re: kde review kartesio Quite Unlikely ... It's a Solver, to fit curves into points, That's very used in any theorical research, engeniering, math, phisics, etc. 2013/5/10 Anne-Marie Mahfouf annemarie.mahf...@free.fr Hi, I am wondering what is the user base for this application as it seems quite specialized (I did not build it yet though). Can you tell us more about the potential target? Another question that comes to mind is: can't it be a feature of an existing KDE Edu apps? Best regards, Anne-Marie - Mail original - De: LucaTringali tringalinv...@libero.it À: kde-core-devel@kde.org Envoyé: Jeudi 9 Mai 2013 18:06:16 Objet: kde review kartesio Hello, I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval. I followed the guidelines ( http://techbase.kde.org/Policies/Application_Lifecycle ) and Kartesio is actually in KDE review: https://projects.kde.org/projects/kdereview/kartesio For any question, ask me. Luca Tringali
Re: Re: kde review kartesio
Hi,
I'll answer point to point:
Messaggio originale
Hey!
A good thing, I think such a tool could be useful to me too (and I
know a lot of other people to whom it might be useful). Here's what I
noticed from a quick look (some has been said already I think):
* You probably shouldn't track the kdev4 file in the repository, same
goes for screenshots
Yes, those files are there just because I found them useful, but I can remove
them without any problem.
* zorbaneural is a very fancy dependency, it's not even in arch's AUR.
You should put the git URL into the cmake message.
I know, I wrote where to find it in Kartesio project overview but, since
almost everybody had some troubles with it, I will make this thing more clear.
* I put x**2 into the fit box and clicked Fit, and it crashed:
http://paste.kde.org/741026/
Yes, the correct way to express a power is ^. So you should write x^2. There
should be a check routine to avoid that a dangerous string like ** is used,
and I'm surely integrating this check in the next release.
* Did you think about laying out the UI around a splitter? On my
screen, the table takes most of the area and the plot is quite small,
and I can't change that...
This could be a good idea: another thing for the next release.
* It would be useful to be able to import data in some way, e.g. from
a CSV file. I don't see a way to get data into the program except
typing every number into the cells -- or is there another way? If
there is, could it be made more obvious eventually?
Actually there is not: I'm working on a new window for the next releases:
basically, there will be a button over the table, something like Edit datas.
This will open a new window in which it will be possible to import/export CSV,
sort X axis values, add other rows or deleting some...
* What does the code in calculations.cpp:117 do? It looks quite
curious. Isn't there a more elegant solution (it looks a bit like a
QChar::isLetter() implementation)?
* calculations.cpp:505 and 584 the same code like in 117 again? It's
weird enough to have that stuff once, but copied multiple times is bad
imho ;)
No, at least not only. Originally, line 117 and 118 were collapsed into one
single if instruction, it slip it in two because it was too long. This line
checks if the current char (which is a C++ char an not a QChar) is permitted or
not. Permitted characters ar letters, numbers, and some other simbols (for
example +, (, etc..).
Could this instruction be shorter and more elagant? Probably. But it works,
and actually I think it could stay as it is.
* Your code uses mixed tab- and space indent (sometimes it uses tabs,
sometimes spaces for no apparent reason). Most KDE apps use only
spaces, you might consider if you want to do that too. Sometimes, the
indent is even missing completely; you should indent one level after
each opening curly parenthesis.
* Same goes for the whole formatting of the code, it's pretty
inconsistent. For example, look at the spaces around operators or so.
I know it, I'll try to make the code more readable, but I do not have so much
time so usually I prefer to dedicate my time to new features or corrections
instead of making them prettier.
* Instead of writing to /tmp/kartesiotmp.txt you should probably use
QTempFile. That will also take care of the deleting the temp file when
it gets deallocated so you don't need to exec (scary and
platform-dependent) rm commands.
I'm already working with QTempFile for the next release of Kartesio.
* calculations.cpp:277 this makes no sense, there's a statement behind
a return
Ooops: I thought I already removed it.
In general, you're mixing a lot of plain C / stdlib stuff into Qt
code. Is there a reason for that? For example, in calculations.cpp:148
you take text from a text field, convert it to a byte array, convert
it to a char* and then pass it to a function. Why not just pass the
QString? You can iterate over a QString like
foreach ( const QChar c, myqstring ) { ... }
or also
for ( int i = 0; i myqstring.size(); i++ ) { ... }
if you like that better, and you can also index it like a char*, as in
mystring[i+1] or so.
Yes, this is an heritage from the older version of Kartesio, that was based
mainly on plain ANSI C++. Those mixing are just an hack to make Kartesio work
immediately. If I'll have time, I will translate everything into Qt, but
first of all I would like to apply other features.
Also, nothing in your code is const and everything is public, although
almost everything could be const and private, but I won't get started
on that now ;)
This is not meant as a list of what you must fix, it's just my two cents.
Thank you, it's always nice to get some suggestions.
Cheers!
Sven
2013/5/10 Tomaz Canabrava tcanabr...@kde.org:
Annma, I find that proposal *very* good.
I'm a bit distant of KDE programming - I know - because my day job is
making
me work 12h+ creating scientific tools.
( actually - one of the tools that I
R: Re: kde review kartesio
Hello everybody, just wanted to tell you that I made some fixes to the code, based on your suggestions: *comments in header files *deleted some unseful string *check routine to avoid that a dangerous string like ** or similar is used for the function *add where to download zorbaneural in cmake module *in neural network algortihm, check if all the points are between 0 and 1 *check if the maxima report is empty before showing it *added file messages.sh I also cleaned it up a little, to make it more readable. Luca Tringali Messaggio originale Da: tringalinv...@libero.it Data: 10/05/2013 14.18 A: kde-core-devel@kde.org, annemarie.mahf...@free.fr Ogg: R: Re: kde review kartesio Hello, libzorbaneural can be found here: https://www.gitorious.org/zorbaneural/zorbaneural/trees/master Here are also some packages for the stable version (deb and rpm): https://www.gitorious.org/zorbaneural/zorbaneural/trees/master/binary- packages/libzorbaneural-0.1 Once you have installed it, the build should go fine. The screenshot folder and the .pro file are not needed, if they are a problem I can remove them. Talking about the system call in calculations, this is the reason why actuallt Kartesio works only on GNU/Linux systems. Why I did it? Because it was the easier way to do that. In the next release of Kartesio, this problem will be solved. I'm not very practical with translatable strings, so I excuse for the Message. sh: is there a wiki page to understand how to write a Message.sh file? I'm writing comments on variables in header files, in the next hours I'll publish them into git. Luca Tringali Messaggio originale Da: annemarie.mahf...@free.fr Data: 10/05/2013 13.58 A: LucaTringalitringalinv...@libero.it Cc: kde-core-devel@kde.org Ogg: Re: kde review kartesio Hi, A few primary remarks: - libzorbaneural is needed but my distro does not have anything with neural in it (OpenSuse 12.3) what repo do I need to add in order to get it? The libzorbaneural website should be added to the cmake file so people can find this and packagers can add it to their distros. - I see a screenshot folder and some .pro files that probably are not needed - some doxygen comments for the variables in the .h files would be appreciated, if anyone else wants to fix bugs it'll help a lot. - Kartesio does not build for me, I get /home/kde- devel/kartesio/src/calculations.cpp:278:1: error: control reaches end of non-void function [-Werror=return-type] cc1plus: some warnings being treated as errors - I don't see a Messages.sh file to extract translatable strings. - I am not comfortable with the rm call line 181 in calculations.cpp = you can probably use more Qt classes here and in other parts of this file too. That's only a quick review as I couldn't run the app yet. Tomaz, as for the user base maybe we could start a module for advanced scientific tools? Best regards, Anne-Marie - Mail original - De: Tomaz Canabrava tcanabr...@kde.org À: Anne-Marie Mahfouf annemarie.mahf...@free.fr Cc: LucaTringali tringalinv...@libero.it, kde-core-devel@kde.org Envoyé: Vendredi 10 Mai 2013 12:28:54 Objet: Re: kde review kartesio Quite Unlikely ... It's a Solver, to fit curves into points, That's very used in any theorical research, engeniering, math, phisics, etc. 2013/5/10 Anne-Marie Mahfouf annemarie.mahf...@free.fr Hi, I am wondering what is the user base for this application as it seems quite specialized (I did not build it yet though). Can you tell us more about the potential target? Another question that comes to mind is: can't it be a feature of an existing KDE Edu apps? Best regards, Anne-Marie - Mail original - De: LucaTringali tringalinv...@libero.it À: kde-core-devel@kde.org Envoyé: Jeudi 9 Mai 2013 18:06:16 Objet: kde review kartesio Hello, I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval. I followed the guidelines ( http://techbase.kde.org/Policies/Application_Lifecycle ) and Kartesio is actually in KDE review: https://projects.kde.org/projects/kdereview/kartesio For any question, ask me. Luca Tringali
kde review kartesio
Hello,I have been working on Kartesio, a program for calculating best fit curves with experimental points. I think it is ready to be moved in the KDE Edu main repo now, so I'm asking your approval.I followed the guidelines (http://techbase.kde.org/Policies/Application_Lifecycle) and Kartesio is actually in KDE review:https://projects.kde.org/projects/kdereview/kartesioFor any question, ask me. Luca Tringali
