pcb

patch to enchance features of NELMA

Bug #1695534 reported by Mike Crowe
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
pcb
Fix Released
Medium
Bert Timmerman

Bug Description

I am updating the NELMA export function to work with gsvit http://gsvit.net/ an open source FDTD solver with GPU computing support. Most of the translation is being done with a separate software package, pcb2gsvit https://github.com/mpcrowe/pcb2gsvit .

The existing code that uses the NELMA output, NELMA-3.2, appears to be dead code and was never fully functional. I haven't tested the code changes against the original NELMA code, but I expect that it breaks it.

The processing chain of (patched)pcb -> pcb2gsvit -> (patched)gsvit is producing an reasonable output. Development is far from complete, but I would like to begin the process of integrating a patch to implement the changes I've implemented so far.

Changes
.png layers now represent unique traces as different colors which map to a netname
an xml file is now generated that provides board specific information.

Remaining work
includes better trace color indexing for netlist mapping to .png
output of component XY(z) data (z being the side of the board component is located)

Rational for implementing patch at this time
Maintains coherent code base (commit early, commit often)
Develop methodology of submitting patches you your group
Begin dialog for developing NELMA export tool

Rational for rejecting patch at this time
Developers are too busy
Patch needs to go in wildly different direction
Patch breaks existing NEMLA tool
Code changes are not complete
Feature patch request needs to be requested with different venue

I feel that the rational for considering this request outweighs the rational for rejecting it. Thank you for considering this request!

Revision history for this message
Mike Crowe (mcrowe) wrote :
Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Thanks for your patch.

Pushed branch LP1695534 with the initial patch to origin for review and test.

Kind regards,

Bert Timmerman.

Changed in pcb:
status: New → Triaged
milestone: none → pcb-4.0.2
importance: Undecided → Medium
Revision history for this message
Chad Parker (parker-charles) wrote :

I don't know really anything about NELMA. I'll have to do some research. Could you prepare some test cases to demonstrate that the exporter is working properly?

Is there any documentation on this format anywhere?

Thanks,
--Chad

Revision history for this message
Dan McMahill (dmcmahill) wrote : Re: [Pcb-bugs] [Bug 1695534] Re: patch to enchance features of NELMA

On 6/19/2017 6:11 PM, Chad Parker wrote:
> I don't know really anything about NELMA. I'll have to do some research.
> Could you prepare some test cases to demonstrate that the exporter is
> working properly?
>
> Is there any documentation on this format anywhere?
>
> Thanks,
> --Chad
>

If the changes are going to break Nelma then perhaps it makes more sense
to create a new HID for the new tool? Or at least have a flag that puts
the HID in nelma mode or gsvit mode.

I'd be interested in seeing an example for how the flow is to work on a
real example.

Revision history for this message
Mike Crowe (mcrowe) wrote :

On 06/19/2017 10:29 PM, Dan McMahill wrote:
> On 6/19/2017 6:11 PM, Chad Parker wrote:
>> I don't know really anything about NELMA. I'll have to do some research.
>> Could you prepare some test cases to demonstrate that the exporter is
>> working properly?
>>
>> Is there any documentation on this format anywhere?
>>
>> Thanks,
>> --Chad
>>
>
> If the changes are going to break Nelma then perhaps it makes more sense
> to create a new HID for the new tool? Or at least have a flag that puts
> the HID in nelma mode or gsvit mode.
>
> I'd be interested in seeing an example for how the flow is to work on a
> real example.
>
Thanks for taking the time to look at my patch! I am still working on
code to integrate data from PCB into gsvit.

Rational for changes
The developer of gsvit suggested that I import the geometry information
of a design to a gsvit specific binary format. I started with the NELMA
export tool, but soon saw that I needed to add additional outputs, and
modify the existing .png outputs. The intermediate software package
https://github.com/mpcrowe/pcb2gsvit takes the modified nelma output and
converts it into this binary format.

Current state of NELMA
The code for NELMA is located here https://www.tablix.org/~avian/nelma/
According to this website a new version of NELMA hasn't been released in
7 years. It is not clear to me how many people are using it. The major
change that might affect nelma is that traces are now assigned a color
based on the net name.

My Opinion
I am an avid user of PCB. It's a great package to use. As an active
user, I feel that adding another HID for the tool adds yet another
feature that users have to think about( cluttering up the export
options). Rather than adding another tool, I would rather add another
flag to implement my patch.

What I am willing to do
I will be happy to resubmit my patch in whatever manner you decide, as a
separate tool, or as a flag option of the existing tool. Please let me
know how you would like to proceed.

Thanks again for considering these changes. I've always wanted to be
able to put my RF pcb designs in to a full blown FDTD simulation. I
think adding this capability will enhance the reputation of what I
already consider to be a great product, PCB.

Cheers
Mike

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi Mike,

I agree with Dan that it would be better to start a new HID for this.

Basically you can do a copy-paste of the nelma-HID and rename (including namespace) to "gsvit".

The snippet in the diff regarding src/find.c should go in a separate commit.

Same for the snippet in src/hid/common/draw_helpers.c.

Please keep commits as small as possible (whitespace!) and specific to one goal.

Kind regards,

Bert Timmerman.

Revision history for this message
Mike Crowe (mcrowe) wrote : Re: [Bug 1695534] Re: patch to enchance features of NELMA

OK I'll move my work into a new hid/gsvit space.

On 07/16/2017 03:48 AM, Bert Timmerman wrote:
> Hi Mike,
>
> I agree with Dan that it would be better to start a new HID for this.
>
> Basically you can do a copy-paste of the nelma-HID and rename (including
> namespace) to "gsvit".
>
> The snippet in the diff regarding src/find.c should go in a separate
> commit.
>
> Same for the snippet in src/hid/common/draw_helpers.c.
>
> Please keep commits as small as possible (whitespace!) and specific to
> one goal.
>
> Kind regards,
>
> Bert Timmerman.
>

Changed in pcb:
milestone: pcb-4.0.2 → pcb-4.1.0
Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

The snippets for src/find.c and src/hid/common/draw_helpers.c are cherry-picked into the master branch upstream.

There are now two topic branches:
- LP1695534 with the patch from Mike,
- home/bert/LP1695534 where I started coding a new gsvit HID, in which
  I will try to include pcb2gsvit code (see the link to github as
  mentioned in #5).

Kind regards,

Bert Timmerman.

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi Mike and all,

I sort of finished reformatting the gsvit HID in "home/bert/LP1695534" and it compiles without warnings.

It needed #include "find.h" and function prototype declarations for:

void common_draw_pcb_line (hidGC gc, LineType *line);
void common_draw_pcb_arc (hidGC gc, ArcType *arc);

in "src/hid/common/draw_helpers.h".

Could you please have look at the code and test functionality please ?

TIA and kind regards,

Bert Timmerman

Revision history for this message
Mike Crowe (mcrowe) wrote :

Hi Bert
Sorry for my late response.
Checked out your branch, compiled and ran it against gsvit2 test1. That
all works. I will test a little more tomorrow.

Thanks for your commitment to this!
Cheers
Mike

On 11/26/2017 02:27 PM, Bert Timmerman wrote:
> Hi Mike and all,
>
> I sort of finished reformatting the gsvit HID in "home/bert/LP1695534"
> and it compiles without warnings.
>
> It needed #include "find.h" and function prototype declarations for:
>
> void common_draw_pcb_line (hidGC gc, LineType *line);
> void common_draw_pcb_arc (hidGC gc, ArcType *arc);
>
> in "src/hid/common/draw_helpers.h".
>
> Could you please have look at the code and test functionality please ?
>
> TIA and kind regards,
>
> Bert Timmerman
>

Changed in pcb:
status: Triaged → In Progress
Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi Mike,

I'm going to dig into pcb2gsvit to see what code is useful for extending the exporter.

It would also be nice to have a test case for the gsvit exporter to perform during a "make distcheck".

Examples of tests can be found in the "tests" directory.

Besides an input file a verified "golden" file is required to check output against.

If you have files suitable for this that would be great.

Kind regards,

Bert Timmerman.

Revision history for this message
Mike Crowe (mcrowe) wrote :

Yes OK I'll make a distcheck.

On 12/05/2017 06:18 PM, Bert Timmerman wrote:
> Hi Mike,
>
> I'm going to dig into pcb2gsvit to see what code is useful for extending
> the exporter.
>
> It would also be nice to have a test case for the gsvit exporter to
> perform during a "make distcheck".
>
> Examples of tests can be found in the "tests" directory.
>
> Besides an input file a verified "golden" file is required to check
> output against.
>
> If you have files suitable for this that would be great.
>
> Kind regards,
>
> Bert Timmerman.
>

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi Mike,

I applied the patches you have send in home/bert/LP1695534.

When exporting a test file I get:

2 colors allocated
holes 1
2 colors allocated
2 colors allocated
*** glibc detected *** /home/bert/workspace/git/pcb/src/pcb: double free or corruption (!prev): 0x090483a0 ***
*** glibc detected *** /home/bert/workspace/git/pcb/src/pcb: malloc(): memory corruption: 0x090483e8 ***

So more things to check before this exporter can be merged into master.

Kind regards,

Bert Timmerman.

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :
Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi Mike,

@@ -1193,12 +1193,12 @@ gsvit_finish_png ()
   Message ("gsvit: PNG not supported by gd. Can't write layer mask.\n");
 #endif
   gdImageDestroy (gsvit_im);
   fclose (gsvit_f);

- free (white);
- free (black);
+// free (white);
+// free (black);
   for (i = 0; i < 0x100; i++) {
     free (color_array[i]);
   }

   gsvit_im = NULL;

Seems to clear the errors ???

Kind regards,

Bert Timmerman.

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi Mike,

Please have a look at the indentation of the drill tags in the attached file "test2_1.xem".

Kind regards,

Bert Timmerman.

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :
Revision history for this message
Mike Crowe (mcrowe) wrote :

The indentation is not perfect, but the xml document is well formed
which is what matters. I didn't want to add a library dependency such
as libxml2 to pcb just to get good indentation.

On 12/30/2017 05:27 PM, Bert Timmerman wrote:
> Hi Mike,
>
> Please have a look at the indentation of the drill tags in the attached
> file "test2_1.xem".
>
> Kind regards,
>
> Bert Timmerman.
>
> ** Attachment added: "test2_1.xem"
> https://bugs.launchpad.net/pcb/+bug/1695534/+attachment/5029258/+files/test2_1.gsvit.xem
>

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi Mike,

I just pushed a commit fixing the indentation (I hope).

Can you run a test on it.

TIA and kind regards,

Bert Timmerman.

Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi Mike,

I just pushed a set of regression tests to "home/bert/LP1695534".

Only awk complaining:

awk: cmd. line:1: /<genTime>Thu Jan 11 23:25:46 2018</genTime>/ {print} "<genTime>today</genTime>"; next}
awk: cmd. line:1: ^ unterminated string
awk: cmd. line:1: /<genTime>Thu Jan 11 23:25:46 2018</genTime>/ {print} "<genTime>today</genTime>"; next}
awk: cmd. line:1: ^ unterminated string

My awk "fu" is weak ;-)

Kind regards,

Bert Timmerman.

Changed in pcb:
assignee: nobody → Bert Timmerman (bert-timmerman)
Revision history for this message
Bert Timmerman (bert-timmerman) wrote :

Hi,

Topic branch "home/bert/LP1695534" now merged into master and pushed upstream.

Kind regards,

Bert Timmerman

Changed in pcb:
status: In Progress → Fix Committed
Changed in pcb:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.