Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hitting max BC limit #467

Open
jvencels opened this issue May 14, 2024 · 23 comments
Open

Hitting max BC limit #467

jvencels opened this issue May 14, 2024 · 23 comments

Comments

@jvencels
Copy link
Contributor

Hi,

Is it possible to let the user define/substitute these parameters in a separate file or as environment variables?

#define DIM 2 /* dimension of the space */
#define MAXDOFS 20 /* maximum number of variables, e.g. T,P */
#define MAXCELLS 100 /* maximum number of subcells in given direction */
#define MAXBOUNDARIES 4000 /* maximum number of boundaries for BCs */
#define MAXCASES 12 /* maximum number of coexisting cases */
#define MAXFILESIZE 600 /* maximum filenamesize for i/o files */
#define MAXLINESIZE 600 /* maximum length of line to be read */
#define LONGLINESIZE 1201
#define MAXNAMESIZE 50 /* maximum size of the variablename */
#define MAXPARAMS 30 /* maximum number of parameters */
#define MAXVARS 20 /* maximum number of variables at the sides */
#define MAXNODESD3 64 /* maximum number of 3D nodes */
#define MAXNODESD2 27 /* maximum number of 2D nodes */
#define MAXNODESD1 9 /* maximum number of 1D nodes */
#define MAXMAPPINGS 20 /* maximum number of geometry mappings */
#define MAXCONNECTIONS 500 /* maximum number of connections in nodal or dual graph */
#define MAXBCS 4000 /* maximum number of BCs in naming */
#define MAXBODIES 1000 /* maximum number of bodies in naming */
#define MAXPARTITIONS 512 /* maximum number of partitions */
#define MAXHALOMODES 10
#define MAXFORMATS 15
#define CONPLAIN 0
#define CONDISCONT 1
#define CONPERIODIC 2
#define CONCONSTRAINT 3
#define MAXELEMENTTYPE 827

As it is a defined variable, it is set before compilation. Although limits are high, we are occasionally hitting max BC limit of 4k. It would be more flexible if variables could be set when starting the executable.

This commit a2ceecc refers to a crash when large BC values are used. If we could test with higher values, we could help to find those issues with a debugger.

@richb2k
Copy link
Contributor

richb2k commented May 15, 2024

Since ElmerGrid is written in C, either define a fixed array size before compilation or use malloc/free to create the array at run time. How many BCs are needed?

A simple approach would be to add a macro guard, to set the max fixed array size to 4000 for Windows and 5000 for all other OSes. If the max number of BCs needed is significantly larger than 5000, then rewriting ElmerGrid to use malloc/free would be required.

@jvencels
Copy link
Contributor Author

Limiting to 4000 does not solve the actual cause of the error. However, we can help find the cause of the error.
It would be awesome to let users change those limits.

@richb2k
Copy link
Contributor

richb2k commented May 15, 2024

Refer to this forum post: [http://www.elmerfem.org/forum/viewtopic.php?t=7978

On Windows, when the max BCs were set to 5000, ElmerGrid crashed due to a stack overflow. Reducing the max BCs to 4000 removed the overflow.

@jvencels
Copy link
Contributor Author

Do you know the line where this overflow happens?

@richb2k
Copy link
Contributor

richb2k commented May 15, 2024

No, it would require recompiling with increased max bcs, and then running a debugger. We know on Windows that the stack overflow happens between 4000 and 5000. We don't know what the upper limit would be under Linux.

@jvencels
Copy link
Contributor Author

jvencels commented May 15, 2024

@raback, can those parameters, or at least MAXBOUNDARIES and MAXBCS, be set during the startup?

Limiting does not sound like a long-term solution. If there is stack overflow, I will help to find the place where it comes from. Nevertheless, we were not having that error.

@richb2k
Copy link
Contributor

richb2k commented May 16, 2024

After recompiling several times and changing the max number of BCs each time, it seems that the offending line is in fempre.c, line 809. On Windows, if max BCs are set to 4900, the call to SaveElmerInput on line 809 succeeds. If the max BCs are set to 5000, the call to SaveElmerInput on line 809 doesn't succeed.

This was tested using wire.grd from the tutorials for the input file: Elmergrid 1 2 wire

@jvencels
Copy link
Contributor Author

Thank you for finding this out. This brings the issue closer to a solution.

@raback
Copy link
Contributor

raback commented May 17, 2024

Code looks rather innocent there.

If I would do things again I would of course allocate all bigger stuff dynamically. However, the code is very old and I was learning C alongside... Making it allocatable is some work.

Over 4000 boundary types sounds quite many. Is the geometry really that complex?

@jvencels
Copy link
Contributor Author

This is a very rare case, but yes - geometry has 6000+ boundaries.
We allow users to set heat transfer boundaries on face groups (like all faces between Core-to-air) and then finetune their setup individually for any face—click on the face, then change type or values.

For this reason, we explode geometry to faces such that they all become a separate heat boundary condition in Elmer. This works well for most cases (usually <100 boundaries).

With new "hands-free" manufacturing technologies, users send CAD masterpieces and want them to be modeled. Although it is a norm to clean geometries before simulating in FEM, those results, according to some, are losing their artistic expression.

Now, we are considering 2 options:

  1. grouping faces to reduce their count
  2. recompiling and hoping for the best

@raback
Copy link
Contributor

raback commented May 17, 2024

I wonder where the MAXBOUNDARIES limit is hit? You see, the Boundary has a integer table types[] that can have any values even though there is just one boundary. So in some sense that ability to have a table of boundaries is redundant. In most cases just one structure is allocated anyways. MAXBOUNDARIES was originally limit of boundaries of the .grd type, I think most parsers lump the boundary elements to just one boundary. Perhaps the limit comes in naming.

So, if you compile the code with MAXBOUNDARIES=100 where does it crash?

@richb2k
Copy link
Contributor

richb2k commented May 17, 2024

I made a few changes to ElmerGrid and ElmerGUI, and was able to increase the maximum number of boundaries to 10,000. Both programs seem to run well on Windows.

kiskot-9507

Attached is an elmergrid input file that generates 9507 boundaries. Change the file suffix from txt back to grd, since github won't load up a grd file directly.
kiskot-9507.txt

@richb2k
Copy link
Contributor

richb2k commented May 18, 2024

Using the Elmer VM with Ubuntu 20, the changes to ElmerGrid worked well. After compiling, running Ctest worked as expected. ElmerGUI is another story with Ubuntu, the same changes done in Windows doesn't work under Ubuntu.

@jvencels
Copy link
Contributor Author

mesh.zip

ElmerGrid 2 2 mesh -metis 6

Fails on Windows and Ubuntu. Once I change MAXBCS to 10000 and recompile, it works on Ubuntu (I did not test on Windows).

I did not change MAXBOUNDARIES; I am not sure if it is necessary.

@raback
Copy link
Contributor

raback commented May 20, 2024

Probably MAXBOUNDARIES could be a lot smaller, maybe 100. This mainly related to .grd format where each BC is its own boundary. For most formats all BCs are included in one boundary.

MAXBCS could maybe be allocated since it is used in so few places.

@raback
Copy link
Contributor

raback commented May 20, 2024

I worked a little with "elmergrid_sizefix" branch. Maybe you can compile that and see if problems are identified in a constrolled manner. There were some inconsistent loop limits, I think. The next step would be to go to dynamic allocation. Have to get back to this later...

@jvencels
Copy link
Contributor Author

Compiled on Ubuntu, it works.

@richb2k
Copy link
Contributor

richb2k commented May 21, 2024

Compiled the elmergrid_sizefix branch in Windows 10. Running the command 'ElmerGrid 2 2 mesh -out mesh2' executed to completion with a warning of

Cannot treat names for boundaries beyond 4000
Number of indexes beyond range: 1767
***** WARNING: This should not happen but we try to continue! *****
***** Omitting use of names because some indexes are beyond range, code some more...

The elmer mesh files were created in the directory mesh2, except the mesh.names was missing.

Changing MAXBCS from 4000 to 10,000 and recompiling and rerunning the same command eliminated the warning and the directory mesh2 contained the six expected elmer mesh files including mesh.names.

Using the version of ElmerGUI generated while compiling elmergrid_fixsize branch, start ElmerGUI, click on 'Load Mesh' and select the either the mesh or the mesh2 directory. ElmerGUI starts to read the mesh and then crashes. This occurs with MAXBCS set to 4000, as well as when set to 10,000.

Run the following command 'ElmerGrid 2 2 mesh -autoclean -out mesh2' and then open ElmerGUI and load the mesh2 mesh directory. The mesh will load successfully, displaying the bodies and boundaries. Note that ElmerGUI has the setting of '-autoclean' applied in the Configure menu, but that doesn't help in this case.

@jvencels
Copy link
Contributor Author

@richb2k Does it mean that it works not worse than before?

@richb2k
Copy link
Contributor

richb2k commented May 23, 2024

Yes, not worse.

I tried setting MAXBCS = 16,000, and that seems to work fine in ElmerGrid and ElmerGUI, as well as still passing in ctest. With the same high setting of MAXBCS, I compiled and ran ctest on both Windows 10 and Ubuntu 20, and the tests passed.

@jvencels
Copy link
Contributor Author

@raback can those changes be merged?

@raback
Copy link
Contributor

raback commented May 29, 2024

I can merge, but maybe with not that big MAXBCS. I guess you can compile bigger setup. What remains to do is make it allocatable. MAXBOUNDARIES are actually only large in ElmerGrid format and it could be probably be made <1000 without any issues. This was a quick half-on-hour-fix so good if it works.

@jvencels
Copy link
Contributor Author

We are using ElmerGrid only to decompose existing Elmer meshes into partitions. After your changes, it was working with default limits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants