Small improvements

Give some feedback on openEMS.

Moderators: thorsten, sebastian

Post Reply
Michael
Posts: 9
Joined: Fri 29 Mar 2019, 18:41

Small improvements

Post by Michael » Fri 29 Mar 2019, 18:54

Hallo Thorsten,

first of all many thanks and a big praise for the OpenEMS project!
Really fantastic indeed.

I will use it for simulating PCBs.
I want to help improving the program and had a look into the code.
Below are the first things that I found.
It would be very nice if you consider them.
Indeed, I want to do much more, but that's the beginning, just to hear how things go.



for function parameters: use pointers to vectors instead of the vector itself, in
FDTD/operator.cpp
Calc_ECPos(), GetMaterial(), Calc_EffMatPos(), AverageMatCellCenter(), AverageMatQuarterCell()
This speeds up the initialization time remarkly (about 2 times).


set default value of "NumberOfTimesteps" to 1000000000,
because then it can be omitted in the XML file, i.e. in
openems.cpp -> void openEMS::Parse_XML_FDTDSetup()
first line "double dhelp=1000000000;" instead of "double dhelp=0;"


Furthermore, I would like to be able to set the max time in XML file, i.e. in
openems.cpp -> void openEMS::Parse_XML_FDTDSetup()
insert
dhelp = 0;
FDTD_Opts->QueryDoubleAttribute("MaxTime",&dhelp);
if (dhelp>0)
this->SetMaxTime(dhelp);


in CSPropMaterial::GetWeight(...) and CSPropExcitation::GetWeightedExcitation(...)
if (coordInputType==1)
replaced by
if (coordInputType == CYLINDRICAL)


CSXCAD/src/CSTransform.cpp
remove
#define PI 3.141592653589793238462643383279
and replace
PI
by
M_PI


CSXCAD/src/CSTransform.cpp
"std::" before "ostream" is missing in
PrintMatrix(std::ostream& stream)
PrintTransformations(std::ostream& stream, std::string prefix)
(gives errors with many compilers)


in order to get rid of the BOOST library:
tools/useful.cpp
remove
#include <boost/algorithm/string.hpp>
replace
SplitString2Float() and SplitString2Double()
by the same function from CSXCAD/src/CSUseful.cpp
but with
val = String2Double(sub, ok);
if (ok)
values.push_back(val);
replaced by
if ( sscanf( sub.c_str(), "%lf", &val ) == 1 )
values.push_back(val);
and
if ( sscanf( sub.c_str(), "%f", &val ) == 1 )
values.push_back(val);
respectively


tools/aligned_allocator.h
tools/array_ops.cpp
insert
#undef __MSVCRT_VERSION__
before #define __MSVCRT_VERSION__
in order to get rid of warnings


CSXCAD/src/ParameterObjects.cpp
in
bool ReadTerm()
remove
double dHelp;
as it's not used


in engine.cpp -> void Engine::Init()
enclose
volt = Create_N_3DArray<FDTD_FLOAT>(numLines);
curr = Create_N_3DArray<FDTD_FLOAT>(numLines);
by
if(m_type != SSE)
and in engine_sse.cpp -> void Engine_sse::Init()
delete
Delete_N_3DArray(volt,numLines);
volt=NULL; // not used
Delete_N_3DArray(curr,numLines);
curr=NULL; // not used

Thanks,
Michael

thorsten
Posts: 1393
Joined: Mon 27 Jun 2011, 12:26

Re: Small improvements

Post by thorsten » Fri 29 Mar 2019, 19:26

Hi Michael,

thanks for the feedback. I will have a closer look at it soon, but it all sounds reasonable.

Do you have experience with git, github and pull-requests?
Ideally you would create commits with your changes and I will review and apply them.
If you do not want to use github you can send me git patches too or you can post your git tree wherever I can pull from.

Doing it like this (using the forum) could be a bit tedious on the long run...


regards
Thorsten

Michael
Posts: 9
Joined: Fri 29 Mar 2019, 18:41

Re: Small improvements

Post by Michael » Fri 29 Mar 2019, 19:40

Hello Thorsten,

wow, that's what I call a fast response!

Unfortunately, I've no experience at all with git, github etc.
I guess I have to register somewhere again. :(
I'm somewhat reluctant doing this so often.
But of course, the forum is not the place for bigger changes.
I'm sure we will find a solution.

Beyond others plans from my side are:
* a command line option to suppress info messages
* speed-up the simulation
* new excitation signals, e.g. DC-free Gaussian pulse

Best regards,
Michael

thorsten
Posts: 1393
Joined: Mon 27 Jun 2011, 12:26

Re: Small improvements

Post by thorsten » Fri 29 Mar 2019, 21:11

Well git as the used version control is of course important I guess if you want to contribute more than an occasional small patch.
And while github makes the pull request and reviewing easy it would not be mandatory. You could publish your changes wherever you want. It would only raise the question how to properly review and maybe iterate the changes...
* new excitation signals, e.g. DC-free Gaussian pulse
That's already possible if you set your center frequency high enough (modulated Gaussian pulse).

Michael
Posts: 9
Joined: Fri 29 Mar 2019, 18:41

Re: Small improvements

Post by Michael » Mon 01 Apr 2019, 15:36

Hello Thorsten,

yes, of course, github is better suited for changes.
Anyway, before spending a lot of time with all these things, I just want to learn how things go here,
whether you agree to my changes, if we have the same opinion, what are the time scales etc.
(Let me know when I'm too annoying, but I have many proposals.)

So concerning to my wishes:
* a command line option to suppress info messages
Can you implement a command line option that put the verbose level to -1?
Can you put an "if (g_settings.GetVerboseLevel() >= 0)" before each info message?
Can you shift the "openEMS::GetExtLibsInfo("\t")" to a higher verbose level or showing it ony "if (argc<=1)"?

* new excitation signals, e.g. DC-free Gaussian pulse
The current pulses are DC free only if the frequencies are high enough.
I want a pulse that is always DC-free with a signal content as close to Dc as possible.
So would you agree to extend the excitation like the following?

void Excitation::CalcGaussianPulsExcitation(double f0, double fc, int nTS)
{
if(fc > 0.0) {
// standard Gaussian pulse whose DC value can be varied by its parameters
......
}
else {
// special Gaussian pulse which is always (=independed from parameters)
// complete DC free, but has a lower useable frequency of about 0.06*f0
......
}
}

thorsten
Posts: 1393
Joined: Mon 27 Jun 2011, 12:26

Re: Small improvements

Post by thorsten » Mon 01 Apr 2019, 19:13

Can you implement a command line option that put the verbose level to -1?
Can you put an "if (g_settings.GetVerboseLevel() >= 0)" before each info message?
Can you shift the "openEMS::GetExtLibsInfo("\t")" to a higher verbose level or showing it ony "if (argc<=1)"?
That should not be to difficult, feel free to do it ;)
* new excitation signals, e.g. DC-free Gaussian pulse
This would need some further discussion, because the modulated (DC free) version of a Gaussian pulse is just that DC free.
Any other shape/form of a pulse is not a Gaussian pulse anymore.
But you are of course free to implement other pulse shapes that better suite your needs. But as far as I see it the Gaussian pulse is in most cases the ideal shape to have. Any sharper pulse in frequency domain always means a much longer signal in time-domain and thus a longer simulation.
To try out new shapes I recommend to first use the Matlab interface to define custom shapes and if you are happy with one implement it as a builtin one.
In general I do not object to have new shapes/pulses, but as I said, it would need/have a different name...

Michael
Posts: 9
Joined: Fri 29 Mar 2019, 18:41

Re: Small improvements

Post by Michael » Tue 02 Apr 2019, 12:27

Hello Thorsten,

what I wanted to ask with all my proposals is essentially:
Will you implement them into the official code?
It's absolutely okay to say "no" or "sorry, no time at the moment" or something similar.
I've just seen that the last update in github was several month ago.
So I guess the project isn't so active at the moment.

Concerning the pulse:
Strictly speaking a Gaussian pulse is exp(t^2), but usually all pulses with f(t)*exp(t^2) are called Gaussian.
cos(t)*exp(t^2) is never completely DC free.
If the simulator waits for an energy decay of 40dB, then DC should be at least -50dB.
With cos(t)*exp(t^2) the bandwidth needs to be very small too reach this.
That's why I want to implement a complete DC-free Gaussian pulse with proper bandwith and with a small lower cut-off freqency.

I hope this makes sense, and if I'm too annoying, please let me know.

Michael
Posts: 9
Joined: Fri 29 Mar 2019, 18:41

Re: Small improvements

Post by Michael » Wed 24 Apr 2019, 09:24

Hello Thorsten,

attached is an archive that contains the above-mentioned changes.
Only changed files are included.
It would be great if you can insert it into the official code.

Many thanks,
Michael
Attachments
CSXCAD-master-new.zip
(22.57 KiB) Downloaded 165 times
openEMS-master-new.zip
(64.95 KiB) Downloaded 167 times

Post Reply