Update ngspice to 43 #25041

bpdegnan · 2024-07-24T15:41:16Z

Description

This update brings ngspice from 36 to 43. I also made myself the maintainer.

Type(s)

bugfix
enhancement
security fix

Tested on

macOS 14.5 23F79 x86_64
Xcode 15.4 15F31d

Verification

Have you

followed our Commit Message Guidelines?
squashed and minimized your commits?
checked that there aren't other open pull requests for the same change?
checked your Portfile with port lint --nitpick?
tried existing tests with sudo port test?
tried a full install with sudo port -vst install?
tested basic functionality of all binary files?
checked that the Portfile's most important variants haven't been broken?

bpdegnan · 2024-07-24T15:42:43Z

my first foray into git and commiting an update.

ryandesign

[X ] followed our Commit Message Guidelines?

You haven't followed the commit message guidelines.

This PR conflicts with master. Somehow you have based your changes off of the January 2022 commit instead of the November 2022 commit.

science/ngspice/Portfile

bpdegnan · 2024-07-24T18:03:14Z

Getting ngspice to build was surprisingly finicky. We might not actually need those. It was sort of all a blur reading everything and trying to get it correct. This is my first time at this, so I expect that I probably botched it as that's the nature of things. We can reject the request and I can start over. (first day on git as well <shrug>)

…

On Wed, Jul 24, 2024 at 1:54 PM Ryan Carsten Schmidt < ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In science/ngspice/Portfile <#25041 (comment)> : > + port:automake \ + port:bison \ I didn't see you add anything to the port that would cause it to actually use autoconf or automake. Are you sure those dependencies are needed? Usually, projects that use autoconf and/or automake have already run those programs and shipped tarballs that include the generated files so that the user (or MacPorts) need not run those programs. — Reply to this email directly, view it on GitHub <#25041 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC5MKZJ3HEX535SLCCYKGUDZN7S4VAVCNFSM6AAAAABLMYKDAKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCOJXGQYDSOJVHE> . You are receiving this because you authored the thread.Message ID: ***@***.***>

science/ngspice/Portfile

pmetzger · 2024-07-28T13:24:55Z

@bpdegnan Do you think you could address the comments above?

bpdegnan · 2024-07-28T14:48:41Z

@bpdegnan Do you think you could address the comments above?

I think that I got it all sorted out.

pmetzger · 2024-07-28T15:47:06Z

@bpdegnan Why did you close the pull request?

bpdegnan · 2024-07-28T16:15:29Z

i did not mean too. i was attempting too change things. this is my first time doing this and between reading the documentation, and not understanding completely how git works, i made some sort of error.

…

On Sun, Jul 28, 2024 at 11:47 Perry E. Metzger ***@***.***> wrote: @bpdegnan <https://github.com/bpdegnan> Why did you close the pull request? — Reply to this email directly, view it on GitHub <#25041 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC5MKZOYJFCPAO6NKEOARFTZOUHBBAVCNFSM6AAAAABLMYKDAKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJUGU3DEMJTGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

pmetzger · 2024-07-28T16:25:04Z

Well, then re-open the request.

bpdegnan · 2024-07-28T16:47:23Z

I reopened it.

…

On Sun, Jul 28, 2024 at 12:25 PM Perry E. Metzger ***@***.***> wrote: Well, then re-open the request. — Reply to this email directly, view it on GitHub <#25041 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC5MKZJSIUOZAWTJWAXA4YDZOULPLAVCNFSM6AAAAABLMYKDAKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJUGU3TEMZTHA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

reneeotten · 2024-08-09T05:15:39Z

reopening is fine but did you take care of all the review comments?

bpdegnan · 2024-08-09T15:04:37Z

I believe that I did address everything, but we might want to just scrap it. As a user of ngspice, most of the tickets are related to how ancient the supported version is in macports, and I thought to solve that as I've solved all of it internally (I have patches too for GCD over openmp). It was a bit of an arduous journey for someone who is not in software to figure out the nuances of what is expected during a pull request, and I believe that it might be better to just scrap the whole thing as I don't want to cause undo stress to people. I read the documentation and still didn't get it right (the linters didn't work when I was using Japanese MacOS for instance, don't know why). I'm going to say that if things look good, we can go with it and if not, let's just scrap it and I'll stick with physics.

pmetzger · 2024-08-13T21:58:36Z

@reneeotten Do you think you could help get this over the hump?

markemer · 2024-08-16T21:54:10Z

What's left? Fixing the commit messages and seeing if one of the OpenMP compilers works?

bpdegnan · 2024-08-17T01:46:26Z

Fixing the commit messages will get you to a functional package that gives the correct results. I have not tested the actual functionality of ngspice-43 with OpenMP because we use the stock compiler that comes with our Macs. The ngspice team has done a great job of making ngspice as good HSPICE and it works on my commercial semiconductor models; however, like with all SPICE, there's some quirks that can exist and we test "the hell out of it". ngspice-43 without OpenMP gives the correct simulation results, even if it's not as fast.

pmetzger · 2024-08-18T18:09:43Z

Conflicts have to be resolved.

Having an OpenMP variant is harmless to those who build without it; we will not get an opportunity to check if it works if there is no easy way to build it.

markemer · 2024-08-22T19:46:03Z

I kinda want to help get this over the line as I would love a modern version of ngspice. Does the MP Clang-18, etc support OpenMP out of the box? I know apples does not, but the notes in the repo show that it can be compiled with OpenMP on a mac using the right compiler. @bpdegnan are you still interested in maintaining this? If so, would you be open to me as a co-maintainer?

bpdegnan · 2024-08-22T20:13:05Z

@markemer I'm totally up for having a co-maintainer. My value is that I actually use the software and know when things aren't correct as we have a suite of circuits that we test. I'm mostly interested in getting a modern ngspice into the ports tree.

pmetzger · 2024-08-23T13:09:35Z

Conflicts need to be resolved btw.

bpdegnan · 2024-08-23T14:28:01Z

I don't understand what the conflicts are honestly as the port file is based on how I was compiling it locally, and I don't consider the lack of openmp a conflict. The openmp issue would require to check things against an openmp compiled ngspice, which I have not done. If someone can suggest one, I can try it out. ngspice is something that I am careful with as older versions very finicky with results. For instance, ngspice-31 doesn't handle the transitions regions well and would give you incorrect results unless you were very careful, and as a reviewer of papers, I can say many were not careful. I realize that openmp should only speed up the solver, but still, I'd like to try it against my commercial models and results. I'm not sure what to do, which i'm willing to help in any way that I can.

markemer · 2024-08-23T22:57:40Z

@pmetzger, @ryandesign

So looking into this, OpenMP is now enabled by default upstream, so I think we should too.

Should I make it a default variant that can be turned off if clang whatever fails to build, or do we a no-openmp variant, do you think?

Also, I have no idea how far up the spec we need - the wiki is a bit out of date which I need to fix, as OpenMP version goes up to like 5.something these days. Do I just keep increasing the number until it compiles? I've only ever done OpenMP for my own stuff, so I just always used the latest.

pmetzger · 2024-08-24T15:37:08Z

@bpdegnan:

I don't understand what the conflicts are honestly as the port file is based on how I was compiling it locally

The meaning of "This branch cannot be rebased due to conflicts" is that some part of the code has been updated in-tree since the pull request was made and conflicts with the changes in the pull request. It has nothing to do with whether you can compile it locally. The pull needs to be updated to accommodate the changes in the in-tree file(s) that changed.

bpdegnan · 2024-08-26T14:16:27Z

@pmetzger I see. I appreciate the detailed description. Let me see if I can sort it out.
@markemer As you have an interest in this and seem to have a better handle on git and pull requests, perhaps we should consider just having you being the maintainer. I feel that I should just stick to semiconductor physics considering what I've made of this so far. You'd have one dedicated user (and my team of others and students) that can just make sure it's all behaving as expected.

markemer · 2024-08-26T16:23:39Z

@pmetzger I see. I appreciate the detailed description. Let me see if I can sort it out. @markemer As you have an interest in this and seem to have a better handle on git and pull requests, perhaps we should consider just having you being the maintainer. I feel that I should just stick to semiconductor physics considering what I've made of this so far. You'd have one dedicated user (and my team of others and students) that can just make sure it's all behaving as expected.

I'm cool with that too - I've had trouble with this port myself. I cut my teeth on 0.18 micron at IBM back in '01 and I've been using ngspice on linux for eFabless 0.13 micron (which is very retro for me) but I've slowly been trying to move the whole workflow including PDKs to MacPorts.

markemer · 2024-08-26T16:27:06Z

I've got a version on OpenMP running using clang17, because that's the earliest I can build on my Apple Silicon machine. What's a good way to require whatever openmp you have? Is is just the open mpversion? Mine seemed to try and install clang 16 at one version, but maybe because I didn't have any OpenMP installed?

bpdegnan · 2024-08-26T18:37:46Z

@pmetzger I believe that I've addressed everything that was pointed out by you and @ryandesign. I'm glad to report that I'm getting better at GIT.
@markemer Let's get this version out, and then lets get the openmp version working. I don't think that I can wrap my brain around anymore changes. This was my first time out with git and a formal process so it's been rather painful for everyone, but I've learned a lot. Also, looking forward, I have some patches that are specific to MacOS and ngspice. As an aside, I'm also using skywater 130nm as I'm hoping to make my designs more "open source". I have access to early node kits, and I have been comparing ngspice results to the commercial equivalents.

markemer · 2024-08-26T20:48:43Z

Looks like you checked in the conflicts. This stuff:

>>>>>>> 931e441f0e3 (nomaintainer science/*: update platforms)
<<<<<<< HEAD
>>>>>>> cc1e90b7a65 (ngspice: update to version 43)

bpdegnan · 2024-08-26T20:56:02Z

@markemer oh sheesh. Let me track that down.

bpdegnan · 2024-08-26T20:59:37Z

@markemer That was me just being an idiot. I pushed the correct one. I had two copies of the trees. I have too many machines, and I lost track of where I was.

markemer · 2024-08-29T18:38:28Z

Cool, I think this is ready to go, lets get this merged as I figure out getting OpenMP working with one of our clangs. I'd be more reluctant, but this version of ngspice is so ancient, I doubt anyone is using it. Been on my list to fix for a while, because I had to build by hand.

cc: @pmetzger let me know what you think - after a timeout, I'll probably just merge it.

science/ngspice/Portfile

pmetzger · 2024-08-29T19:18:58Z

The file now seems mangled. I think a bad commit happened.

bpdegnan · 2024-08-29T20:15:00Z

I don't know how to unmangle this, and I'm not sure what to do. As I obviously cannot figure this out, I say we just scrap the whole thing. @markemer would be a better maintainer as he's obviously up on git, whereas I just cannot get out of my own way. I should just stick with physics. Any thoughts here? Should we just scrap this all and wait for Mark to get the openmp version working?

markemer · 2024-08-29T20:19:53Z

The file now seems mangled. I think a bad commit happened.

Is it mangled? The latest seems fine to me and it builds on CI.

markemer · 2024-08-29T20:27:12Z

@pmetzger I pushed a change, see if there is still an issue. I don't see anything on my end.

pmetzger · 2024-08-29T21:33:17Z

Doesn't look mangled any more.

reneeotten · 2024-08-30T05:59:01Z

to whoever merges this when all issues are resolved: make sure to squash the commits as the history is a mess now 😉

pmetzger · 2024-08-30T14:12:34Z

Someone needs to squash and rebase before anything proceeds anyway.

bpdegnan · 2024-08-30T16:56:43Z

I will attempt to squash and rebase. I figure, it can get any worse than I previously made it.

bpdegnan · 2024-08-30T17:45:33Z

I did some squashing and rebasing and if I did it right, i squashed everything up to just be a single update. It looks like it might have pulled it off. I read a summary of squashing so I'm hoping things didn't go sideways as this was my first foray into fixing problems that I caused. it all appears to have gone swimmingly, but let me know please.

markemer · 2024-08-30T18:34:11Z

I did some squashing and rebasing and if I did it right, i squashed everything up to just be a single update. It looks like it might have pulled it off. I read a summary of squashing so I'm hoping things didn't go sideways as this was my first foray into fixing problems that I caused. it all appears to have gone swimmingly, but let me know please.

Looks good to me - letting it run now.

Edit: Actually my one commit got dropped, probably because you didn't have it since it was the last one. I'll add that back in, squash and merge this evening.

Closes: https://trac.macports.org/ticket/56552 Closes: https://trac.macports.org/ticket/70175 Closes: https://trac.macports.org/ticket/70435

barracuda156 · 2024-08-31T02:37:25Z

How was this even supposed to work, forget fixing tickets? :)
Expectedly, it now fails at configure itself.

barracuda156 · 2024-08-31T03:13:11Z

Why was the patch removed? Nothing has been fixed in the codebase, it still fails:

get_avail_mem_size.c:22:2: warning: #import is a deprecated GCC extension [-Wdeprecated]
   22 | #import <mach/mach.h>
      |  ^~~~~~
get_resident_set_size.c: In function 'getCurrentRSS':
get_resident_set_size.c:114:33: error: storage size of 'info' isn't known
  114 |     struct mach_task_basic_info info;
      |                                 ^~~~
get_avail_mem_size.c:23:2: warning: #import is a deprecated GCC extension [-Wdeprecated]
   23 | #import <mach/mach_host.h>
      |  ^~~~~~
get_resident_set_size.c:115:40: error: 'MACH_TASK_BASIC_INFO_COUNT' undeclared (first use in this function); did you mean 'TASK_BASIC_INFO_COUNT'?
  115 |     mach_msg_type_number_t infoCount = MACH_TASK_BASIC_INFO_COUNT;
      |                                        ^~~~~~~~~~~~~~~~~~~~~~~~~~
      |                                        TASK_BASIC_INFO_COUNT
get_resident_set_size.c:115:40: note: each undeclared identifier is reported only once for each function it appears in
get_resident_set_size.c:116:40: error: 'MACH_TASK_BASIC_INFO' undeclared (first use in this function); did you mean 'TASK_BASIC_INFO'?
  116 |     if ( task_info( mach_task_self( ), MACH_TASK_BASIC_INFO,
      |                                        ^~~~~~~~~~~~~~~~~~~~
      |                                        TASK_BASIC_INFO
get_resident_set_size.c:116:5: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
  116 |     if ( task_info( mach_task_self( ), MACH_TASK_BASIC_INFO,
      |     ^~
get_resident_set_size.c:119:9: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if'
  119 |         return (unsigned long long) info.resident_size;
      |         ^~~~~~
get_resident_set_size.c:114:33: warning: unused variable 'info' [-Wunused-variable]
  114 |     struct mach_task_basic_info info;
      |                                 ^~~~
get_resident_set_size.c:140:1: warning: control reaches end of non-void function [-Wreturn-type]
  140 | }
      | ^
make[4]: *** [get_resident_set_size.lo] Error 1

macportsbot added type: update maintainer: none labels Jul 24, 2024

ryandesign requested changes Jul 24, 2024

View reviewed changes

pmetzger reviewed Jul 27, 2024

View reviewed changes

science/ngspice/Portfile Outdated Show resolved Hide resolved

pmetzger reviewed Jul 27, 2024

View reviewed changes

science/ngspice/Portfile Show resolved Hide resolved

bpdegnan closed this Jul 28, 2024

bpdegnan reopened this Jul 28, 2024

markemer requested review from ryandesign and pmetzger August 29, 2024 18:37

pmetzger reviewed Aug 29, 2024

View reviewed changes

science/ngspice/Portfile Outdated Show resolved Hide resolved

pmetzger reviewed Aug 29, 2024

View reviewed changes

science/ngspice/Portfile Outdated Show resolved Hide resolved

ngspice: update to version 43

850a060

Closes: https://trac.macports.org/ticket/56552 Closes: https://trac.macports.org/ticket/70175 Closes: https://trac.macports.org/ticket/70435

markemer merged commit bd31f2c into macports:master Aug 30, 2024
3 checks passed

ryandesign mentioned this pull request Aug 31, 2024

ngspice: fix the port for < 10.8 #25537

Merged

12 tasks

bpdegnan deleted the update-ngspice branch September 3, 2024 01:39

Update ngspice to 43 #25041

Update ngspice to 43 #25041

Conversation

bpdegnan commented Jul 24, 2024 • edited by markemer Loading

Description

Type(s)

Tested on

Verification

bpdegnan commented Jul 24, 2024

ryandesign left a comment

Choose a reason for hiding this comment

bpdegnan commented Jul 24, 2024 via email

pmetzger commented Jul 28, 2024

bpdegnan commented Jul 28, 2024

pmetzger commented Jul 28, 2024

bpdegnan commented Jul 28, 2024 via email

pmetzger commented Jul 28, 2024

bpdegnan commented Jul 28, 2024 via email

reneeotten commented Aug 9, 2024

bpdegnan commented Aug 9, 2024

pmetzger commented Aug 13, 2024

markemer commented Aug 16, 2024

bpdegnan commented Aug 17, 2024

pmetzger commented Aug 18, 2024

markemer commented Aug 22, 2024

bpdegnan commented Aug 22, 2024

pmetzger commented Aug 23, 2024

bpdegnan commented Aug 23, 2024

markemer commented Aug 23, 2024

pmetzger commented Aug 24, 2024

bpdegnan commented Aug 26, 2024

markemer commented Aug 26, 2024

markemer commented Aug 26, 2024

bpdegnan commented Aug 26, 2024

markemer commented Aug 26, 2024

bpdegnan commented Aug 26, 2024

bpdegnan commented Aug 26, 2024

markemer commented Aug 29, 2024 • edited Loading

pmetzger commented Aug 29, 2024

bpdegnan commented Aug 29, 2024

markemer commented Aug 29, 2024

markemer commented Aug 29, 2024

pmetzger commented Aug 29, 2024

reneeotten commented Aug 30, 2024

pmetzger commented Aug 30, 2024

bpdegnan commented Aug 30, 2024

bpdegnan commented Aug 30, 2024

markemer commented Aug 30, 2024 • edited Loading

barracuda156 commented Aug 31, 2024

barracuda156 commented Aug 31, 2024

bpdegnan commented Jul 24, 2024 •

edited by markemer

Loading

markemer commented Aug 29, 2024 •

edited

Loading

markemer commented Aug 30, 2024 •

edited

Loading