Open szaghi opened 11 years ago
Start to merge @victorsndvg work from its fork.
There are a lot of work to do...
Victor programming styles seems too much different from mine. Take this in count.
The library source is becoming huge. Consider to exploit include
for making light the source file.
Speak of this with victor.
Victor XML parsing seems complex: pay attention to the auxiliary procedures he is using.
Speak of this with victor.
Victor adopted a new set of public interface where the suffix _READ
means importer procedures. I was thinking to hide the difference between importers and exporters into the name with something like:
! importer
E_IO = VTK_INI_XML(...,action='load')
! export
E_IO = VTK_INI_XML(...,action='save')
Speak of this with victor.
Hi Stefano,
I'm glad to see that you are reviewing the new and big piece of code... Good luck! :)
Feel free to ask or suggest anything. I will try to help you with any doubt and I can also try to adapt or reimplement the code according to your suggestions. Probably it will be a long conversation :)
Best regards, Víctor.
@victorsndvg
Thank you very much for your kindness, it is very appreciated.
Let's go!
In your version, that is the most up-to-date, the lines counting is now 12572
: too much for me. I am thinking to exploiting include
statement with the following logic:
the procedures of (let us say) VTK_GEO_XML
will go into Lib_VTK_IO-GEO_XML.inc
thus the actual implementations of the
module procedure VTK_GEO_XML_STRG_1DA_R8, VTK_GEO_XML_STRG_3DA_R8, & ! real(R8P) StructuredGrid, 1D/3D Arrays
VTK_GEO_XML_STRG_1DAP_R8,VTK_GEO_XML_STRG_3DAP_R8, & ! real(R8P) StructuredGrid, 1D/3D Arrays packed API
VTK_GEO_XML_STRG_1DA_R4, VTK_GEO_XML_STRG_3DA_R4, & ! real(R4P) StructuredGrid, 1D/3D Arrays
VTK_GEO_XML_STRG_1DAP_R4,VTK_GEO_XML_STRG_3DAP_R4, & ! real(R4P) StructuredGrid, 1D/3D Arrays packed API
VTK_GEO_XML_RECT_R8, & ! real(R8P) RectilinearGrid
VTK_GEO_XML_RECT_R4, & ! real(R4P) RectilinearGrid
VTK_GEO_XML_UNST_R8,VTK_GEO_XML_UNST_PACK_R4, & ! real(R8P) UnstructuredGrid, standard and packed API
VTK_GEO_XML_UNST_R4,VTK_GEO_XML_UNST_PACK_R8, & ! real(R4P) UnstructuredGrid, standard and packed API
VTK_GEO_XML_CLOSEP
will go into Lib_VTK_IO-GEO_XML.inc
thus the procedures definitions that are now after the main module contains
will be replaced with a simple
include("Lib_VTK_IO-GEO_XML.inc")
The same for all big generic interface.
What do you think about this?
@szaghi
I've never used the include statement before. I have to read something about it.
My first impression, and baseless opinion, about include
statement is that it seems a very old-school-F77 style. May be i'm wrong, sorry for my impudence :)
And, why not to modularize the code?
I mean, reorganize the source code in different modules. For example:
File Lib_VTK_IO-GEO_XML.f90
should contain the module Lib_VTK_IO-GEO_XML_module
with all the subroutines and interfaces for both reading and writing, or only for one of them.
File Lib_VTK_IO.f90
should use all the modules and work like a front-end for the rest of modules.
What do you think ?
@victorsndvg
You are (almost) right.
include
is in standard from very old time (I am not sure but I think that f66 should have). However, it is still not included into the deprecated feature, because it has its pros:
include
avoid the compiling cascades hierarchy that comes with use module...
; it is not a real problem, I have FoBiS.py :-) but other users may complain about;include
means literally that the contents of the file are inserted where is the include statement, actually replacing the statement itself.Indeed, my first thought was (as your) to use module for packing the interfaces. However, besides the new compiling hierarchy (that is a minor issue), my main concern is about the (ab)use of some global variables defined into the Lib_VTK_IO main module: if we use (sub)modules for packing the interfaces, we must ensure to avoid circular modules use because interface-modules must access to the global variables defined into the main module and the main module must use the interface-modules... if we agree to modularize the library, we must plan a module also for global variable (being careful with multi-thread security...).
Ok, tomorrow I will propose a possible modularization and a possible include exploiting trying to highlight pros and cons of both.
See you soon.
@szaghi ,
it's nice to learn reading your posts. Thank you.
I'm agree with you, there must be some modules (or only 1) containing global variables and common procedures. As an early/basic idea, and according to this, to avoid circular dependencies between modules we could think in a 3 (or more) level hierarchy. The lowest level could contain those common and global things and the highest level work as a front-end.
The biggest work should the placed in the intermediate levels, containing the interfaces an procedures for effective reading and writing.
The highest level will use the intermediates level modules and, following the hierarchy, those levels will use the lowest level.
Do you think this is possible? Do you see any potential problem of this simple schema?
@victorsndvg
You are completely right. This night, "sloshing" my daughter, I realized that modularization is definitely the better approach because allow the easy maintenance of the library and favorite the future refactoring (I am planning to refactor the API to allow OOP, that, besides other things, means encapsulate all...). I think to discard the idea of exploiting include
facility.
Thus we can think to our modularization. I like to try implement the modularization step by step with you. The following is just a first idea.
Three levels of abstraction (as you suggest):
No idea... do you have some suggestions?
Today I will create a new branch, beside the master one, named importers. I would like to allow you to directly control with me my gitrepo, thus you can control my mistakes... is it ok for you?
See you soon.
@victorsndvg
Just added a new branch, importers.
The modularization is done as above described.
The naming convention adopted is trivial, but feel free to suggest a more smart one.
Into the back-end module I have already inserted your auxiliary procedures. Besides some minor changes for conforming my (bad) style, I have made some major changes:
This is my break-lunch work.
See you soon.
@szaghi
an OOP approach will be great!
I'm completely agree with you.
About modules naming, as a user of your library since some time ago, i think that the actual name functions are friendly. I think that maintaining this names will help the old users to adapt and understand the new modularization.
Maybe something like:
It's Ok for me. I will follow the changes too near to understand your refactoring improvements
See you.
@victorsndvg
Just added you as collaborators. You can push your commit directly here.
See you.
@szaghi
About the two previous point talking about auxiliary procedures:
12.6.1 Argument Correspondence ... Keyword actual argument specification can aid readability by decreasing the need to remember the precise sequence of dummy arguments in dummy argument lists. The order-independence also facilitates more flexible use of optional arguments by not re- quiring omitted arguments to be at the end of the list. These functionalities, and the forms that they take, are modeled after keyword specifiers in input/output statements. ... A procedure reference may use a mixture of positional and keyword correspon- dence, specifying keywords for some actual arguments but not for others. In this case, all the actual arguments without keywords must appear prior to those with keywords in the actual argument list. ...
It seems that, talking about ambiguous signatures, the order of those mixed arguments affect only to the procedure calling if you don't use the keyword.
I'm not sure if I answered your question, I did?
See you
@victorsndvg
You are right. I was thinking exactly to the sentences you have reported (I had not the standard under my eyes today). I use named associations always alsi for avoid ambiguity. However, the sentence you reported is not talking about the definition of the procedure (as I was concerning) rather to the caller signature. Tomorrow I will check, but lrt us now consider an example:
call foo(a=1,b=2)
call foo(1,b=2)
call foo(1,2)
call foo(1)
The eventual ambiguity comes from:
As a consequence, the following definition are not identical:
subroutine foo(a,b)
integer, optional:: a
integer::b
subroutine foo(b,a)
integer::b
integer, optional:: a
The problem is that I don remember which of the above two is better.
Anyhow, tomorrow should be easy to check.
Night!
@szaghi
It's hard to read all references to optional arguments in the handbook to get what we want to know. Now I'm going to continue reading.
I think that is strongly coherent to put the non optional arguments first, because when you call the procedures without keywords: ...arguments without keywords must appear prior... . I always use it in this way because I think that it's usual in Python.
I hope to get it more clear today
@victorsndvg
My today tests have been usefulness. I cannot discover why (if I remember right) some Intel's Fortran Guru showed me that it was a best practice define optional arguments before non optional....
On the contrary, re-reading a great book, Modern Fortran, style and usage of Clerman and Spector, I found our searching best practice hint
Consistently place subprogram arguments in the following order: the pass argument, intent(in out) arguments, intent(in) arguments, intent (out) arguments, optional arguments.
If you agree, I will follow this convention.
@szaghi
sorry I cannot found any useful info ...
I'm agree with you. I will take note of this convention also for my future works
Importers are missing.