ochubar / Radia

3D Magnetostatics Computer Code
Other
35 stars 19 forks source link

Does this project accept contributions? #2

Open per-gron opened 4 years ago

per-gron commented 4 years ago

Hi,

I'm trying to use Radia in a project I'm working on. While working on integrating Radia with my project (which is in C++ fyi) I have run into some issues with Radia's code, at this point mostly near-mechanical C++ code quality issues, some of which have required me to fork the Radia code, which I would like to avoid if at all possible.

Does Radia accept contributions? I would like to upstream my fixes.

Here is a list of issues that I have found:

Surface-level code issues:

Issues that are a bit more work to fix:

Points marked with X are the ones that are most important to me personally as they are the ones that I feel require me to fork the code. I cannot use Radia without fixing them. It would be great if you would accept fixes at least these, or even better for the "surface level code issues" list. I'm not sure I will have time to actually work on the second list.

ochubar commented 4 years ago

Hi, Thanks for trying to use Radia, the 3D Magnetostatics code that is in use by Community since 1997. I appreciate your comments, and will try to look into some of them, other priorities permitting. Here are a few general "comments on your comments".

per-gron commented 4 years ago

Thanks for the response!

I completely understand that Radia is an old project with a legacy, and the requirement to be cross platform. I appreciate this aspect of the library, as I am able to run it on Linux now, and might later compile it to run in browsers with webassembly. As I mentioned, I use Clang (which is used by Xcode) now, but I have worked on code bases which target all of the compilers you mention at once.

Do you know which versions of those C++ compilers that you are targeting? The features that I most would like to use, in particular unique_ptr were introduced in C++11, which is two entire C++ standards old. Even Visual C++ which traditionally has lagged behind quite a bit has had full C++11 support for a long time now.

Regarding the memory leaks: I'll send some PRs with fixes, I have fixed 3 so far. Address sanitizer is a very accurate tool which is designed to never report an error unless there definitely is a bug.

When I mentioned tests I meant automated tests. It is generally accepted in the software industry that it is very important to write programs that run the code and test various aspects of the code. Such test can be unit tests, integration tests etc. They take some time to write in the first place, but for pieces of software that are meant to be maintained over time, they save a lot of effort in the long run.

BTW, why can't you use the Radia C API and a compiled library in your project, instead of modifying the C++ files?

Building the Radia code from source makes certain things much easier, including debugging, running sanitizers for finding bugs, porting to different platforms etc. The project I'm working on uses many libraries and building everything from source helps keep the complexity down.

The changes I have done to Radia code are rather unobtrusive.

ochubar commented 4 years ago

Hi, I develop on Windows, always using the ~most recent version of Visual C++ (currently in VS 2019, community edition), and then compile on GCC, and after this on Xcode (often with someone's help). I am a physicist (very busy with multiple scientific projects, not a software engineer or computer scientist), and I'm sure I woduld never manage to develop Radia and my other codes - for the amount of time I spent on this - if I wouldn't use Visual C++ as my main dev. tool.

Regarding the memory leaks: I'll send some PRs with fixes, I have fixed 3 so far.

I'm looking forward to seeing these. I didn't face issues with memory leaks in Radia over the last ~20 years :), ...but that's probably because I'm mainly using only a small subset of functions; and there may be issues with functions that were added ~recently (i.e. after 2006 :)).

I wonder, what's your background, and why do you need Radia (I know a few colleagues at PSI in Switzerland who like and make use of Radia since a while, but you seem to be from a different area?).

When I mentioned tests I meant automated tests. It is generally accepted in the software industry that it is very important to write programs that run the code and test various aspects of the code...

In physics, validity / accuracy test of a simulation code usually consists in comparing its simulation results with measurements made on real systems / devices; the other aspects are often much less important. :) …But I appreciate your comments and maybe assistance in the software engineering aspects, if these will result in improvements of the code and maybe productivity.

O.Chubar tenure physicist National Synchrotron Light Source II Brookhaven National Lab, USA

per-gron commented 4 years ago

I develop on Windows, always using the ~most recent version of Visual C++ (currently in VS 2019, community edition), and then compile on GCC, and after this on Xcode (often with someone's help)

Ok! Visual C++ has supported almost all (including unique_ptr) of C++11 since version 2015, and GCC/Clang have supported C++11 pretty much since the standard was published. If you don't need to support old compilers (for example because you're targeting old versions of Windows or embedded devices with proprietary compiler toolchains) I think you should be able to use C++11 with no issues. C++11 introduced several features that make life much easier.

I didn't face issues with memory leaks in Radia over the last ~20 years :) [...]

The memory leaks I have found are not big ones. I care about them mostly because I want to use Address Sanitizer for everything, and it's much easier to see regressions if there are zero errors.

I wonder, what's your background,

I'm a software engineer. I currently work at Google, and before that I worked at Spotify. I don't do any science-y or math-y work there, but have done a lot of C++.

and why do you need Radia (I know a few colleagues at PSI in Switzerland who like and make use of Radia since a while, but you seem to be from a different area?).

I am working on (not for Google) a project that involves simulating the behavior of electric guitar pickups, which are magnetic sensors. Radia is designed to do much more advanced things than this, but it is the most suitable piece of software that I have been able to find. I need to simulate magnetic fields in 3D of permanent magnets.

In physics, validity / accuracy test of a simulation code usually consists in comparing its simulation results with measurements made on real systems / devices; the other aspects are often much less important. :) …But I appreciate your comments and maybe assistance in the software engineering aspects, if these will result in improvements of the code and maybe productivity.

I see. Since it's not obvious if there is an error by just looking at the output things get a bit more tricky than typical software. I think some tests that just run as much of the code lines as possible that don't actually look at the answer, but that can be run with sanitizers, could help catch plenty of basic mistakes such as array out of bounds accesses etc. If you can get people who use this software to contribute code that runs their experiments, it could make sense to have these as tests that verify that the output after a code change doesn't change too much compared to the output that was there before (this only works for simulations that run in reasonable amounts of time, perhaps <30 minutes each maybe, ideally <10 mins).

per-gron commented 4 years ago

How is Radia compiled with Xcode/Mac? Do you use make? I can't find an Xcode project (except ext_lib/igor/XOPSupport/Xcode/XOPSupport.xcodeproj which doesn't seem to contain the whole project).

per-gron commented 4 years ago

Would you be interested in a Pull Request that integrates Radia with Travis CI?

Travis is an online tool. With very little configuration it can be configured so that for every pull request and every commit to the master branch it will build the code (and run tests if they exist) and report the results back in the PR. It supports Linux, Windows and Mac, so it can be set up to compile every change on every platform and compiler toolchain that Radia supports. And it's free for open source projects.

If you are currently checking that the code works on different operating systems manually, it seems like this could save you quite a bit of time.

ochubar commented 4 years ago

Hi! Thanks for the pull requests.

Visual C++ has supported almost all (including unique_ptr) of C++11 since version 2015,...

It's a too short term on my scale.:) I have a habit (and pleasure) programming via pointers, and often need advanced memory management, so I'll wait until pointers will be announced "deprecated" in C++ (if this will happen before I stop programming :)). Thank you for pointing me to these C++ updates. It is certainly good to see how it evolves.

I am working on (not for Google) a project that involves simulating the behavior of electric guitar pickups, which are magnetic sensors. Radia is designed to do much more advanced things than this, but it is the most suitable piece of software that I have been able to find. I need to simulate magnetic fields in 3D of permanent magnets.

Interesting. Note however that Radia is a magnetostatics code; calculating static 3D magnetic field is easy with it, but as is, it won't allow you to directly simulate time-/frequency-dependent phenomena (e.g. electromagnetic induction or Eddy currents); some "post-processing" may need to be done in your case. We with colleagues keep in mind extending Radia in these directions, benefiting from fast solving of static / steady-state problems (especially after we implement parallelization), but this is a relatively big effort (that may require a good motivation).

If you can get people who use this software to contribute code that runs their experiments, it could make sense to have these as tests that verify that the output after a code change doesn't change too much compared to the output that was there before (this only works for simulations that run in reasonable amounts of time, perhaps <30 minutes each maybe, ideally <10 mins).

We use standard Radia examples for this purpose (there are ~7 in the Mathematica and Python versions). I know what their results should be, but nothing is automated. Perhaps something can be done on this basis(?); any references will be strongly appreciated!

How is Radia compiled with Xcode/Mac? Do you use make?

We did not yet update the Xcode project to compile a recent Radia shared / static library and Py version for Mac; there is only an out-of-dated Xcode project for compiling Radia for Mathematica, but we plan to update it. The easiest at this point would probably be to take the Makefile for Linux and modify it for Mac.

dhidas commented 4 years ago

Just to note, I was able to compile the latest radia on os x 10.15 (latest xcode) for python (3.7 and others) and mathematica 12. In order to do this I had to comment out:

include

and _FPU_SETCW(cword); From cpp/src/ext/triangle/triangle.c

And made some modifications to the makefiles. Oleg, I am happy to pass along the modifications and/or binaries.

I do really like the idea of travis. Also, for python users it would be great to use pypi.

Best,

-Dean

On 12/30/19, 5:33 PM, "Oleg Chubar" notifications@github.com wrote:

Hi!
Thanks for the pull requests.

Visual C++ has supported almost all (including unique_ptr) of C++11 since version 2015,...

It's a too short term on my scale.:) I have a habit (and pleasure) programming via pointers, and often need advanced memory management, so I'll wait until pointers will be announced "deprecated" in C++ (if this will happen before I stop programming :)).
Thank you for pointing me to these C++ updates. It is certainly good to see how it evolves.

I am working on (not for Google) a project that involves simulating the behavior of electric guitar pickups, which are magnetic sensors. Radia is designed to do much more advanced things than this, but it is the most suitable piece of software that I have
 been able to find. I need to simulate magnetic fields in 3D of permanent magnets.

Interesting. Note however that Radia is a magnetostatics code; calculating static 3D magnetic field is easy with it, but as is, it won't allow you to directly simulate time-/frequency-dependent phenomena (e.g. electromagnetic induction or Eddy currents);
 some "post-processing" may need to be done in your case. We with colleagues keep in mind extending Radia in these directions, benefiting from fast solving of static / steady-state problems (especially after we implement parallelization), but this is a relatively
 big effort (that may require a good motivation).

If you can get people who use this software to contribute code that runs their experiments, it could make sense to have these as tests that verify that the output after a code change doesn't change too much compared to the output that was there before (this
 only works for simulations that run in reasonable amounts of time, perhaps <30 minutes each maybe, ideally <10 mins).

We use standard Radia examples for this purpose (there are ~7 in the Mathematica and Python versions). I know what their results should be, but nothing is automated. Perhaps something can be done on this basis(?); any references will be strongly appreciated!

How is Radia compiled with Xcode/Mac? Do you use make?

We did not yet update the Xcode project to compile a recent Radia shared / static library and Py version for Mac; there is only an out-of-dated Xcode project for compiling Radia for Mathematica, but we plan to update it. The easiest at this point would probably
 be to take the Makefile for Linux and modify it for Mac.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, 
view it on GitHub <https://github.com/ochubar/Radia/issues/2?email_source=notifications&email_token=ABMBVUHRQ5FL6ZHYHKROOB3Q3JZJNA5CNFSM4KAJTMP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH3LERY#issuecomment-569815623>, or 
unsubscribe <https://github.com/notifications/unsubscribe-auth/ABMBVUGEWG26DRVKA7CFG23Q3JZJNANCNFSM4KAJTMPQ>.
[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/ochubar/Radia/issues/2?email_source=notifications\u0026email_token=ABMBVUHRQ5FL6ZHYHKROOB3Q3JZJNA5CNFSM4KAJTMP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH3LERY#issuecomment-569815623",
"url": "https://github.com/ochubar/Radia/issues/2?email_source=notifications\u0026email_token=ABMBVUHRQ5FL6ZHYHKROOB3Q3JZJNA5CNFSM4KAJTMP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH3LERY#issuecomment-569815623",
"name": "View Issue"
},
"description": "View this Issue on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]
ochubar commented 4 years ago

Would you be interested in a Pull Request that integrates Radia with Travis CI?

Thanks already for the reference! Let me first check with our guys in the lab; I thinks they mentioned this thing too (I'd like to be "compatible").

per-gron commented 4 years ago

I have a habit (and pleasure) programming via pointers,

Pointers are not going away ever in C++. With modern C++ features like unique_ptr the pointers are still there, it's just that the compiler does a lot of the tedious but difficult work for you. For example, the data() method on std::vector gives you a raw pointer to the underlying array, but std::vector gives you nice things like automatic memory management, the ability to resize the array, and it automatically calls destructors and similar on its elements. It's got all the nice things of the C++ of old, but it does things like writing the code in radTInteraction::EmptyVectOfPtrToListsOfTrans() automatically for you, and it doesn't make off-by-one errors.

It takes a little bit of time to learn how to use the new features, but some of them, especially the ones around automatic memory management, are well worth it. (Others, like template metaprogramming features are not necessary to know most of the time.) When you don't have to worry about mechanical things like when to write delete you are free to focus on other things that are specific to your application.

and often need advanced memory management,

I think you'd be surprised by how much ground is covered by unique_ptr, shared_ptr, vector and unordered_map/set. And the nice thing is: If you do need something more advanced, like arena allocators, graphs with cycles etc, you can still write them exactly like before, just don't use these classes then.

so I'll wait until pointers will be announced "deprecated" in C++

You might be interested to see that the C++ Core Guidelines which is a set of code style guidelines written by Bjarne Stroustrup and published by the Standard C++ Foundation recommend to not call new and delete explicitly.

ochubar commented 4 years ago

Hi Dean, thanks for the update. I wonder, does triangulation still work after the changes you had to do? Perhaps you can check this with Example #7 on Mathematica (or I can send you its Py version soon)? If all examples seem to work fine - don't hesitate to make a pull request.

per-gron commented 4 years ago

If you can make the Makefiles work on macOS, it should be easy to make it run on Travis. I don't have easy access to a Mac right now so it is a bit difficult for me to tweak the makefiles to work there.

We use standard Radia examples for this purpose (there are ~7 in the Mathematica and Python versions). I know what their results should be, but nothing is automated. Perhaps something can be done on this basis(?); any references will be strongly appreciated!

Using the examples as a small automated test suite sounds like a good idea to me! If you want the tests to be written in Python, you should be able to use the unit testing framework that is built into Python. It's also possible to write the tests in C or C++.

Automated tests (at least the kind that runs on a single computer) are at the core usually nothing more than a command line program that exits with code 0 on success and non-0 on failure. Then you have some kind of test runner that runs these programs and tells you if any of the tests failed.

Note however that Radia is a magnetostatics code; [...]

In the project I'm working on, running the simulations in real-time is an absolute requirement, accuracy is not. Because of this, even if Radia gets support for solving non-static problems, I don't think it would be directly useful for me. I want to use Radia to compute the magnetic field of a guitar pickup "at rest" from a 3D model of permanent magnets and iron/steel bars/slugs. For the actual time-domain processing we will need to resort to very simplified models that use the static "at rest" data as input.

dhidas commented 4 years ago

You can compare the results from os x mma 12 for example 7: https://hidas.org/temp/Example7/

If you don't see any problem I will attempt to formulate some pull request with the makefile in a week or so (shutdown busyness slows me down a bit..). If others want to test the osx version I can pass along the binaries (maybe marco?).

As a side note, I was interested in viewing radia models in jupyter (for linux and os x). I did a few things in ipyvolume and k3d and found these quite useful (and fast) for this type of rendering. I'm not sure if there are any efforts on this front, but I think it would be great for the pyradia version and I found them quite easy to use. So far I was parsing the output of UtiDmp, but I'm not sure that it gives all of the transformation information that is needed. It may be possible to write a c function which puts this in a nice format for one of these renderers. Just an idea I was thinking about recently...

Best,

-Dean

On 12/30/19, 5:58 PM, "Oleg Chubar" notifications@github.com wrote:

Hi Dean, thanks for the update.
I wonder, does triangulation still work after the changes you had to do? Perhaps you can check this with Example #7 on Mathematica (or I can send you its Py version soon)? If all examples seem to work fine - don't hesitate to make a pull request.
—
You are receiving this because you commented.
Reply to this email directly, 
view it on GitHub <https://github.com/ochubar/Radia/issues/2?email_source=notifications&email_token=ABMBVUBKKMR2D4KVWOHZ7HDQ3J4KNA5CNFSM4KAJTMP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH3MNIA#issuecomment-569820832>, or 
unsubscribe <https://github.com/notifications/unsubscribe-auth/ABMBVUHKOGGA7YKTFWJX77TQ3J4KNANCNFSM4KAJTMPQ>.
[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/ochubar/Radia/issues/2?email_source=notifications\u0026email_token=ABMBVUBKKMR2D4KVWOHZ7HDQ3J4KNA5CNFSM4KAJTMP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH3MNIA#issuecomment-569820832",
"url": "https://github.com/ochubar/Radia/issues/2?email_source=notifications\u0026email_token=ABMBVUBKKMR2D4KVWOHZ7HDQ3J4KNA5CNFSM4KAJTMP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH3MNIA#issuecomment-569820832",
"name": "View Issue"
},
"description": "View this Issue on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]
robnagler commented 4 years ago

@dhidas we have developed a VTK.js integration for Radia and converted RADIA_Example05

It's on our beta server now, but as soon as it is approved (probably next week), we'll move it to jupyter.radiasoft.org.

It is available as a Docker image (alpha or beta tags) with a curl installer:

curl https://jupyter.run | bash -s beta

Docker needs to be installed first.

youzheho commented 3 years ago

Dear all:

I have one issue that i cannot run the radia om mac os catalina(version.10.15.5)

Did anyone who can help me step by step? my email is youzheho@gmail.com Thanks You Zhed

lufermar commented 1 year ago

Has anyone solved the problem that youzheho had? I also cannot execute Radia.exe on Mac and I would appreciate some help:)

robnagler commented 1 year ago

@lufermar you can use Radia via Docker. RadiaSoft offers several public images with Radia (and many other codes) builtin:

You can also access Radia online for free via Sirepo Radia, which is a graphical front-end for Radia. RadiaSoft's free JupyterHub server allows you to run Radia via the command line or a notebook.

Compiling Radia on the Mac will likely require some expertise with Homebrew. You can probably install the compilers and then look over RadiaSoft's build script for Linux. At RadiaSoft, we use Macs, and run Radia via VirtualBox, JupyterHub, and Docker. We do not compile on our Macs directly.