-
Notifications
You must be signed in to change notification settings - Fork 6
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
init branch with slight changes to the code in gapc.cc and signature.cc #219
Conversation
Der Test mit der Datei aliglob2.gap lief soweit super. Da kommt ein Sinniger Graph raus für den input AC DC. Ansonsten muss eigentlich nurnoch der Output in eine Datei redirected werden und der Body hinzugefügt werden. |
kannst Du statt der PDF einen Screenshot machen und hier als IMGage direkt einbetten? |
Zwei Dinge:
|
Hier handelt es sich um einen MultiTrack Algorithmus (jede Eingabe = 1 Track, hier also zwei). Das habe ich bei der Grammatikvisualisierung immer durch einzelne Zeilen visualiert, z.B: oder hier in der ADP Vorlesung Bekommst Du das mit TikZ auch hin? |
src/location.hh
Outdated
@@ -0,0 +1,332 @@ | |||
// A Bison parser, made by GNU Bison 3.5.1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ebenfalls auto generated
Ich hoffe jetzt sieht man das Bild dann direkt!
Okay, das erklärt die Symbole die ich gesucht habe, super! Dann sollte ich das Problem gefunden haben, ich sitze drann!
Das sollte eigentlich machbar sein! Das würde ich mir dann anschauen wenn die RNA-Folds sauber generiert werden. Dann müsste ich meine ich nur noch mit ein paar Flags bei den einzelnen nodes auch Farben setzen können.
Das habe ich total vergessen! Beim nächstem Push denke ich daran! |
src/alt.cc
Outdated
@@ -1306,6 +1306,7 @@ void Alt::Multi::print(std::ostream &s) { | |||
s << " > "; | |||
} | |||
|
|||
// ret_ variablen in out.cc hier veränderbar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Englische Code-Kommentare sind meist sinnvoller - gerade bei einem weltöffentlichen Repository
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Englische Code-Kommentare sind meist sinnvoller - gerade bei einem weltöffentlichen Repository
Ich versuche es generell auf Englisch zu halten, manchmal sind das kurze Randnotizen für mich. Wird abgeänder!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
besides German text, I feel this comment does not fully explain what's happening. The function creates a new Variable Declaration Statement for the generated target language independent code with the name prefix
+ ret_
+ i.
Yes, you might change the hard coded ret_
part here, but I am not sure if this won't break code generation at other places. Thus, I'd remove this comment here completely and consider it only a hint for you while working with the code base.
src/cpp.cc
Outdated
@@ -2149,7 +2152,7 @@ void Printer::Cpp::print_stats_fn(const AST &ast) { | |||
|
|||
void Printer::Cpp::header_footer(const AST &ast) { | |||
dec_indent(); | |||
stream << indent() << " private:" << endl; | |||
stream << indent() << " private:" << endl; // printed unter bei cyk() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kommentar auf englisch verfassen und evtl anpassen (aussagekräftiger beschreiben)
src/gapc.cc
Outdated
@@ -692,6 +698,8 @@ class Main { | |||
Printer::Cpp hh(driver.ast, opts.h_stream()); | |||
hh.set_argv(argv, argc); | |||
hh.class_name = opts.class_name; | |||
// opts.product == enum_graph einfach polymorphe header methode schreiben |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s. oben
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like a dirty hack :-) What is the use of this check for equality?
src/prefix.cc
Outdated
#include "prefix.hh" | ||
|
||
namespace gapc { | ||
const char prefix[] = "/homes/dtischle/Projekte"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hart codierte Pfade sind immer schlecht. geht das anders? Oder ist das hier auto-generiert? dann sollte das sowieso nicht im Repo mit drin sein
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Das sollte autogeneriert sein, bei den nächsten Pushes achte ich mal mehr darauf das cleane Projekt zu pushen!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
genau :-) Datei bitte aus PR entfernen, da autogeneriert
src/symbol.cc
Outdated
@@ -861,7 +861,7 @@ void Symbol::NT::add_specialised_arguments(Statement::Fn_Call *fn, | |||
|
|||
void Symbol::NT::set_ret_decl_rhs(Code::Mode mode) { | |||
ret_decl = new Statement::Var_Decl(data_type_before_eval(), | |||
new std::string("answers")); | |||
new std::string("answer")); //changes all answers in out.cc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
es wäre evtl sinnvoll zu schreiben wie es verändert wird.
src/version.txt
Outdated
@@ -0,0 +1 @@ | |||
2023.07.05 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
kannst du evtl als Code-Kommentar hinzufügen in welchem Format das Datum hier steht? Also yyyy/mm/dd oder das englische Format yyyy/dd/mm ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Setze ich mich demnächst drann! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
stopp. Diese Datei wird automatisch erzeugt. Bitte nicht anfassen :-) und auch aus dem Repo rausnehmen!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wenn sie autogeneriert wird, weißt du dann wie dieses Datum codiert ist? ist das Mai oder Juli? :D
Das erste generierte Bild sieht schonmal gut aus. Damit ich das verstehe: Wird hier für den einen Kandidaten nur ein Kandidatenbaum ausgegeben? Wie bekommt ein User das ausgegeben? Als Datei oder über die Konsole? Kann man das speichern? In welchem Dateiformat? |
Aktuell erzeugt die Autoalgebra für jeden möglichen Kandidaten einen Tikz Code. Da fehlt jetzt nur noch ein Header und Footer für TexMaker, das mache ich aktuell noch per Hand. Ich habe jetzt nochmal eine Version gepushed, in der zwischen Single und Multitrack unterschieden wird. Nächster Step wäre dann die Einbettung in eine .tex Datei. Stefan meinte er wollte alle Graphen als Subgraphen in einer Datei haben, da würde ich mal schauen ob das Funktioniert, sonst würde ich für jeden Graphen einen Output generieren. Eventuell mit Flag der den Output begrenzt wenn es zu viele werden. Da weiß ich aber noch nicht genau, wie das anzustellen ist. |
Achso. Output soll eine Datei werden die über Tex in ein pdf umgewandelt werden kann. |
src/version.cc
Outdated
@@ -0,0 +1,5 @@ | |||
#include "version.hh" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Datei bitte aus PR entfernen, da autogeneriert
src/stack.hh
Outdated
@@ -0,0 +1,8 @@ | |||
// A Bison parser, made by GNU Bison 3.5.1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Datei bitte aus PR entfernen, da autogeneriert
src/position.hh
Outdated
@@ -0,0 +1,11 @@ | |||
// A Bison parser, made by GNU Bison 3.5.1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Datei bitte aus PR entfernen, da autogeneriert
src/parser.output
Outdated
@@ -0,0 +1,6119 @@ | |||
Terminals unused in grammar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Datei bitte aus PR entfernen, da autogeneriert
src/parser.hh
Outdated
@@ -0,0 +1,829 @@ | |||
// A Bison parser, made by GNU Bison 3.5.1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Datei bitte aus PR entfernen, da autogeneriert
src/parser.cc
Outdated
@@ -0,0 +1,3116 @@ | |||
// A Bison parser, made by GNU Bison 3.5.1. | |||
|
|||
// Skeleton implementation for Bison LALR(1) parsers in C++ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Datei bitte aus PR entfernen, da autogeneriert
src/options.hh
Outdated
@@ -212,6 +212,7 @@ struct Options { | |||
return *plotgrammar_stream_; | |||
} | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please revert
499dc4c
to
999a77e
Compare
much better. Thanks! |
src/alt.cc
Outdated
@@ -1306,6 +1306,7 @@ void Alt::Multi::print(std::ostream &s) { | |||
s << " > "; | |||
} | |||
|
|||
// ret_ variablen in out.cc hier veränderbar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
besides German text, I feel this comment does not fully explain what's happening. The function creates a new Variable Declaration Statement for the generated target language independent code with the name prefix
+ ret_
+ i.
Yes, you might change the hard coded ret_
part here, but I am not sure if this won't break code generation at other places. Thus, I'd remove this comment here completely and consider it only a hint for you while working with the code base.
src/cpp.cc
Outdated
@@ -1033,7 +1033,7 @@ void Printer::Cpp::print_eqs(const std::list<Statement::Var_Decl*> &l, char c) { | |||
} | |||
|
|||
|
|||
void Printer::Cpp::print_most_decl(const Symbol::NT &nt) { | |||
void Printer::Cpp::print_most_decl(const Symbol::NT &nt) { // header instance variables in class out for resulting out.hh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Printer functions generate code for either *.cc or *.hh files. Depends on the value of fwd_decls
, thus this comment is not appropriate in the general case. You see why documentation is sparse ... it is complicated ;-)
@@ -2506,9 +2509,8 @@ void Printer::Cpp::print_value_pp(const AST &ast) { | |||
stream << indent() << "} else {" << endl; | |||
inc_indent(); | |||
stream << indent() << "out << res << '\\n';" << endl; | |||
dec_indent(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this a general error? But if I look at the line below, we are closing the block via }
. Why should we increase instead of decrease the indentation?
@@ -368,6 +368,9 @@ class Main { | |||
driver.ast.set_outside_nt_list(&opts.outside_nt_list); | |||
driver.parse_product(opts.product); | |||
|
|||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please revert empty lines
src/gapc.cc
Outdated
@@ -692,6 +698,8 @@ class Main { | |||
Printer::Cpp hh(driver.ast, opts.h_stream()); | |||
hh.set_argv(argv, argc); | |||
hh.class_name = opts.class_name; | |||
// opts.product == enum_graph einfach polymorphe header methode schreiben |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like a dirty hack :-) What is the use of this check for equality?
src/parser.y
Outdated
@@ -764,7 +764,8 @@ algebra: algebra_head '{' fn_defs '}' | |||
\begin{lstlisting} | |||
automatic_specifier: | |||
@enum@ | | |||
@count@ | |||
@count@ | | |||
@enum_graph@ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel we need to come up with a proper name. Currently we have auto algebra functions for enum
and count
. Therefore, I'd suggest a one word name. How about trees
(especially with the plural to emphasize that you expect to obtain many trees, i.e. one per candidate)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like a dirty hack :-) What is the use of this check for equality?
That's a fragment and will be deleted
src/signature.cc
Outdated
if (*mode == "enum_graph") | ||
return generate_enum_graph(n); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename to trees
?
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert
src/signature.hh
Outdated
@@ -97,6 +97,7 @@ class Signature : public Signature_Base { | |||
private: | |||
Algebra *generate_count(std::string *n); | |||
Algebra *generate_enum(std::string *n); | |||
Algebra *generate_enum_graph(std::string *n); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enum_graph
-> trees
testdata/config-tests/config.mf
Outdated
@@ -1,281 +0,0 @@ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
jetzt hast Du zuviel gelöscht :-) Diese Datei bitte wieder rein!
…algebra without the need to touch the generic main, since i use another print function in gapcc.cc depending of opts.product. Header and Footer and on the make. Colors will follow
src/cpp.cc
Outdated
stream << indent() << "file_stream << \"\\\\begin{document} \" << std::endl;" << endl; | ||
stream << indent() << "file_stream << \"\\\\begin{tikzpicture}\" << std::endl;" << endl; | ||
stream << indent() << "file_stream << \"\\\\node {root} \" << std::endl;" << endl; // Each Root generates a subplot; Need to name them individually so they can be generated all in one file. Should be possible since the root name won't need any senseful name | ||
stream << indent() << "file_stream << res;" << endl; // TO-DO -> split res on "\n"(?) so we can generate a node entry for each entry in res |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any way for me to manipulate the contents of res?
If i'm able to get all separate lines of res i should be able to generate subplots in one file for all trees. But since this is a List i dont know how to properly access the contents. So far im trying to find a function that i could use inside of rtlib/rope.hh, so far without any progress.
…nside a new file, we shouldn't need it anymore
…r each candidate. Then the output should be pipeable into a .tex file.
Hi @Mighty0r0n I myself have some issues to properly run all the tests at the moment. Two issues are blocking:
Assuming the above will be fixed, you might want to take a look at my PR #221 for inspiration. This is only focusing on creating a tikZ compatible string=Rope. How this is integrated into an output is currently unclear to me. |
Closing in favor of #221 |
Added first auto algebra implementation with loose logic