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

Parameters: Use AMReX Parser #792

Open
wants to merge 2 commits into
base: development
Choose a base branch
from

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Jan 9, 2025

Use the advanced right-hand-side parser of AMReX to read user parameters in inputs files. Enable constants, temporary variables, reuse of parameters, and simple mathematical expressions.

Examples: Use Parser Where Sensible.

@ax3l ax3l added changes input scripts / defaults Changes the syntax or meaning of input scripts and/or defaults component: core Core ImpactX functionality component: elements Elements/external fields labels Jan 9, 2025
@ax3l ax3l requested review from WeiqunZhang and cemitch99 January 9, 2025 21:44
@@ -8,10 +8,10 @@ beam.charge = 1.0e-9
beam.particle = electron
beam.distribution = waterbag
beam.lambdaX = 3.162277660e-6
beam.lambdaY = 3.162277660e-6
beam.lambdaY = beam.lambdaX
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to change all of the existing examples? Couldn't this cause some confusion to users? It might be better to have just one example to illustrate this feature.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it whenever it made sense so people come definitely across it. I hope I did not add it in confusing places, can revert a few where it is complicating the logic ofc.

Copy link
Member

@cemitch99 cemitch99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one question/comment about the examples.

@ax3l ax3l force-pushed the topic-query-add-parser branch 3 times, most recently from 9da3ad0 to 4aa0a49 Compare January 10, 2025 02:43
@ax3l
Copy link
Member Author

ax3l commented Jan 10, 2025

There is some funny behavior for queryAddWithParser/queryWithParser that throws off the Python tests... Behaves somehow like get or so....

Segfaults here
image

@ax3l ax3l force-pushed the topic-query-add-parser branch from 4aa0a49 to 68efd71 Compare January 10, 2025 02:59
@WeiqunZhang
Copy link
Member

WeiqunZhang commented Jan 11, 2025

I think the issue with python is ParmParse::Initialize (called by amrex::Initialize) has not been called when ImpactX::init_grids is called.

@WeiqunZhang
Copy link
Member

We could try to make amrex::ParmParse more robust so that queryWithParse would work even without ParmParse::Initialize. But it may not be that easy to make it thread safe.

@WeiqunZhang
Copy link
Member

Nevertheless, if we assume queryWithParser (before ParmParse::initialize is called) is not used inside omp parallel, it's easy to fix.

@WeiqunZhang
Copy link
Member

However, it may not make sense to use queryWithParser if ParmParse has not been initialized. For example

foo.x = 1
foo.y = foo.x

does not work as expected if foo.x and foo.y are not in the ParmParse database.

@WeiqunZhang
Copy link
Member

AMReX-Codes/amrex#4291 I did the FODO.py.run test, it seems to work with the amrex PR.

@ax3l
Copy link
Member Author

ax3l commented Jan 12, 2025

Could I call ParmParse::initialize before amrex::Initialize to make this example work?
#792 (comment)

We def use ParmParse quite a bit before AMReX init (WarpX: dim selection; ImpactX: configuration, ...).

What actually happens in MPI contexts if we do that? Usually, ParmParse reads files only from the I/O rank, and in that case...?

@ax3l ax3l mentioned this pull request Jan 12, 2025
ax3l added 2 commits January 11, 2025 22:27
Use the advanced right-hand-side parser of AMReX to read user
parameters in inputs files. Enable constants, temporary variables,
reuse of parameters, and simple mathematical expressions.
@ax3l ax3l force-pushed the topic-query-add-parser branch from 68efd71 to 62795b5 Compare January 12, 2025 06:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes input scripts / defaults Changes the syntax or meaning of input scripts and/or defaults component: core Core ImpactX functionality component: elements Elements/external fields
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants