Skip to content

[experimental] Curved-element meshing and Palace 3D photonics example #131

Open
mdmaas wants to merge 10 commits into
gdsfactory:mainfrom
EpsilonForge:palace_photonics
Open

[experimental] Curved-element meshing and Palace 3D photonics example #131
mdmaas wants to merge 10 commits into
gdsfactory:mainfrom
EpsilonForge:palace_photonics

Conversation

@mdmaas
Copy link
Copy Markdown
Contributor

@mdmaas mdmaas commented May 11, 2026

This PR implements spline-fitting to replace curve loops, which enables curved-element meshing.

This is potentially interesting for several PDKs with curved features (like QPDK) but maybe most interesting for 3D photonics, for which Palace could lead to better performance than Meep.

@mdmaas mdmaas changed the title [draft] Curved-element meshing and Palace photonics example and [experimental] Curved-element meshing and Palace photonics example and May 11, 2026
@mdmaas mdmaas changed the title [experimental] Curved-element meshing and Palace photonics example and [experimental] Curved-element meshing and Palace 3D photonics example May 11, 2026
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces support for patterned dielectric layers and high-order geometric mesh elements in Palace simulations. Key features include the ability to disable synthetic dielectric regions, the implementation of spline and bspline curve fitting for dielectric boundaries, and the integration of high-order elements into the meshing and visualization pipelines. A new example notebook for a directional coupler is provided; however, feedback indicates that the current validation logic incorrectly triggers a ValueError for purely dielectric simulations, which is a critical issue for photonic component modeling.

Comment thread nbs/palace_dc.ipynb Outdated
Comment on lines +309 to +333
"cell_type": "code",
"execution_count": 7,
"id": "59a9840b",
"metadata": {},
"outputs": [
{
"ename": "ValueError",
"evalue": "Mesh validation failed:\nValidation: FAILED\nErrors:\n - No conductor surfaces in mesh. Check that conductor layers have polygons and correct layer_type.\n - config.json has no Conductivity or PEC boundaries.\nWarnings:\n - Volumes: ['air', 'core']",
"output_type": "error",
"traceback": [
"\u001b[31m---------------------------------------------------------------------------\u001b[39m",
"\u001b[31mValueError\u001b[39m Traceback (most recent call last)",
"\u001b[36mCell\u001b[39m\u001b[36m \u001b[39m\u001b[32mIn[7]\u001b[39m\u001b[32m, line 1\u001b[39m\n\u001b[32m----> \u001b[39m\u001b[32m1\u001b[39m results = sim.run()\n",
"\u001b[36mFile \u001b[39m\u001b[32m~/Desktop/gsim/src/gsim/palace/driven.py:128\u001b[39m, in \u001b[36mDrivenSim.run\u001b[39m\u001b[34m(self, parent_dir, verbose, wait)\u001b[39m\n\u001b[32m 115\u001b[39m \u001b[38;5;250m\u001b[39m\u001b[33;03m\"\"\"Run the driven sim on GDSFactory+ cloud.\u001b[39;00m\n\u001b[32m 116\u001b[39m \n\u001b[32m 117\u001b[39m \u001b[33;03mThin wrapper over :meth:`PalaceSimMixin.run` that narrows the\u001b[39;00m\n\u001b[32m (...)\u001b[39m\u001b[32m 124\u001b[39m \u001b[33;03m ``job_id`` string when ``wait=False``.\u001b[39;00m\n\u001b[32m 125\u001b[39m \u001b[33;03m\"\"\"\u001b[39;00m\n\u001b[32m 126\u001b[39m \u001b[38;5;28;01mfrom\u001b[39;00m\u001b[38;5;250m \u001b[39m\u001b[34;01mgsim\u001b[39;00m\u001b[34;01m.\u001b[39;00m\u001b[34;01mpalace\u001b[39;00m\u001b[34;01m.\u001b[39;00m\u001b[34;01mresults\u001b[39;00m\u001b[38;5;250m \u001b[39m\u001b[38;5;28;01mimport\u001b[39;00m SParams \u001b[38;5;28;01mas\u001b[39;00m _SParams\n\u001b[32m--> \u001b[39m\u001b[32m128\u001b[39m result = \u001b[30;43msuper\u001b[39;49m\u001b[30;43m(\u001b[39;49m\u001b[30;43m)\u001b[39;49m\u001b[30;43m.\u001b[39;49m\u001b[30;43mrun\u001b[39;49m\u001b[30;43m(\u001b[39;49m\u001b[30;43mparent_dir\u001b[39;49m\u001b[30;43m,\u001b[39;49m\u001b[30;43m \u001b[39;49m\u001b[30;43mverbose\u001b[39;49m\u001b[30;43m=\u001b[39;49m\u001b[30;43mverbose\u001b[39;49m\u001b[30;43m,\u001b[39;49m\u001b[30;43m \u001b[39;49m\u001b[30;43mwait\u001b[39;49m\u001b[30;43m=\u001b[39;49m\u001b[30;43mwait\u001b[39;49m\u001b[30;43m)\u001b[39;49m\n\u001b[32m 129\u001b[39m \u001b[38;5;28;01mif\u001b[39;00m \u001b[38;5;28misinstance\u001b[39m(result, (_SParams, \u001b[38;5;28mstr\u001b[39m)):\n\u001b[32m 130\u001b[39m \u001b[38;5;28;01mreturn\u001b[39;00m result\n",
"\u001b[36mFile \u001b[39m\u001b[32m~/Desktop/gsim/src/gsim/palace/base.py:1430\u001b[39m, in \u001b[36mPalaceSimMixin.run\u001b[39m\u001b[34m(self, parent_dir, verbose, wait)\u001b[39m\n\u001b[32m 1390\u001b[39m \u001b[38;5;28;01mdef\u001b[39;00m\u001b[38;5;250m \u001b[39m\u001b[34mrun\u001b[39m(\n\u001b[32m 1391\u001b[39m \u001b[38;5;28mself\u001b[39m,\n\u001b[32m 1392\u001b[39m parent_dir: \u001b[38;5;28mstr\u001b[39m | Path | \u001b[38;5;28;01mNone\u001b[39;00m = \u001b[38;5;28;01mNone\u001b[39;00m,\n\u001b[32m (...)\u001b[39m\u001b[32m 1395\u001b[39m wait: \u001b[38;5;28mbool\u001b[39m = \u001b[38;5;28;01mTrue\u001b[39;00m,\n\u001b[32m 1396\u001b[39m ) -> SParams | \u001b[38;5;28mdict\u001b[39m[\u001b[38;5;28mstr\u001b[39m, Path] | \u001b[38;5;28mstr\u001b[39m:\n\u001b[32m 1397\u001b[39m \u001b[38;5;250m \u001b[39m\u001b[33;03m\"\"\"Run simulation on GDSFactory+ cloud.\u001b[39;00m\n\u001b[32m 1398\u001b[39m \n\u001b[32m 1399\u001b[39m \u001b[33;03m Requires mesh() to be called first. Automatically calls\u001b[39;00m\n\u001b[32m (...)\u001b[39m\u001b[32m 1428\u001b[39m \u001b[33;03m >>> print(results[\"eig.csv\"])\u001b[39;00m\n\u001b[32m 1429\u001b[39m \u001b[33;03m \"\"\"\u001b[39;00m\n\u001b[32m-> \u001b[39m\u001b[32m1430\u001b[39m \u001b[30;43mself\u001b[39;49m\u001b[30;43m.\u001b[39;49m\u001b[30;43mupload\u001b[39;49m\u001b[30;43m(\u001b[39;49m\u001b[30;43mverbose\u001b[39;49m\u001b[30;43m=\u001b[39;49m\u001b[30;43;01mFalse\u001b[39;49;00m\u001b[30;43m)\u001b[39;49m\n\u001b[32m 1431\u001b[39m \u001b[38;5;28mself\u001b[39m.start(verbose=verbose != \u001b[33m\"\u001b[39m\u001b[33mquiet\u001b[39m\u001b[33m\"\u001b[39m)\n\u001b[32m 1432\u001b[39m \u001b[38;5;28;01mif\u001b[39;00m \u001b[38;5;129;01mnot\u001b[39;00m wait:\n",
"\u001b[36mFile \u001b[39m\u001b[32m~/Desktop/gsim/src/gsim/palace/base.py:1322\u001b[39m, in \u001b[36mPalaceSimMixin.upload\u001b[39m\u001b[34m(self, verbose)\u001b[39m\n\u001b[32m 1308\u001b[39m \u001b[38;5;250m\u001b[39m\u001b[33;03m\"\"\"Prepare config, upload to the cloud. Does NOT start execution.\u001b[39;00m\n\u001b[32m 1309\u001b[39m \n\u001b[32m 1310\u001b[39m \u001b[33;03mRequires :meth:`set_output_dir` and :meth:`mesh` to have been\u001b[39;00m\n\u001b[32m (...)\u001b[39m\u001b[32m 1318\u001b[39m \u001b[33;03m or :func:`gsim.wait_for_results`.\u001b[39;00m\n\u001b[32m 1319\u001b[39m \u001b[33;03m\"\"\"\u001b[39;00m\n\u001b[32m 1320\u001b[39m \u001b[38;5;28;01mfrom\u001b[39;00m\u001b[38;5;250m \u001b[39m\u001b[34;01mgsim\u001b[39;00m\u001b[38;5;250m \u001b[39m\u001b[38;5;28;01mimport\u001b[39;00m gcloud\n\u001b[32m-> \u001b[39m\u001b[32m1322\u001b[39m tmp = \u001b[30;43mself\u001b[39;49m\u001b[30;43m.\u001b[39;49m\u001b[30;43m_prepare_upload_dir\u001b[39;49m\u001b[30;43m(\u001b[39;49m\u001b[30;43m)\u001b[39;49m\n\u001b[32m 1323\u001b[39m \u001b[38;5;28;01mtry\u001b[39;00m:\n\u001b[32m 1324\u001b[39m \u001b[38;5;28mself\u001b[39m._job_id = gcloud.upload(tmp, \u001b[33m\"\u001b[39m\u001b[33mpalace\u001b[39m\u001b[33m\"\u001b[39m, verbose=verbose)\n",
"\u001b[36mFile \u001b[39m\u001b[32m~/Desktop/gsim/src/gsim/palace/base.py:1295\u001b[39m, in \u001b[36mPalaceSimMixin._prepare_upload_dir\u001b[39m\u001b[34m(self)\u001b[39m\n\u001b[32m 1292\u001b[39m \u001b[38;5;28;01mraise\u001b[39;00m \u001b[38;5;167;01mValueError\u001b[39;00m(\u001b[33m\"\u001b[39m\u001b[33mOutput directory not set. Call set_output_dir() first.\u001b[39m\u001b[33m\"\u001b[39m)\n\u001b[32m 1294\u001b[39m \u001b[38;5;66;03m# Always (re)generate config.json to reflect current driven settings\u001b[39;00m\n\u001b[32m-> \u001b[39m\u001b[32m1295\u001b[39m \u001b[30;43mself\u001b[39;49m\u001b[30;43m.\u001b[39;49m\u001b[30;43mwrite_config\u001b[39;49m\u001b[30;43m(\u001b[39;49m\u001b[30;43m)\u001b[39;49m\n\u001b[32m 1297\u001b[39m \u001b[38;5;66;03m# Copy input files to a temp dir so we don't destroy the user's directory\u001b[39;00m\n\u001b[32m 1298\u001b[39m tmp = Path(tempfile.mkdtemp(prefix=\u001b[33m\"\u001b[39m\u001b[33mpalace_\u001b[39m\u001b[33m\"\u001b[39m))\n",
"\u001b[36mFile \u001b[39m\u001b[32m~/Desktop/gsim/src/gsim/palace/base.py:1272\u001b[39m, in \u001b[36mPalaceSimMixin.write_config\u001b[39m\u001b[34m(self)\u001b[39m\n\u001b[32m 1270\u001b[39m validation = \u001b[38;5;28mself\u001b[39m.validate_mesh()\n\u001b[32m 1271\u001b[39m \u001b[38;5;28;01mif\u001b[39;00m \u001b[38;5;129;01mnot\u001b[39;00m validation.valid:\n\u001b[32m-> \u001b[39m\u001b[32m1272\u001b[39m \u001b[38;5;28;01mraise\u001b[39;00m \u001b[38;5;167;01mValueError\u001b[39;00m(\u001b[33mf\u001b[39m\u001b[33m\"\u001b[39m\u001b[33mMesh validation failed:\u001b[39m\u001b[38;5;130;01m\\n\u001b[39;00m\u001b[38;5;132;01m{\u001b[39;00mvalidation\u001b[38;5;132;01m}\u001b[39;00m\u001b[33m\"\u001b[39m)\n\u001b[32m 1274\u001b[39m \u001b[38;5;28;01mreturn\u001b[39;00m config_path\n",
"\u001b[31mValueError\u001b[39m: Mesh validation failed:\nValidation: FAILED\nErrors:\n - No conductor surfaces in mesh. Check that conductor layers have polygons and correct layer_type.\n - config.json has no Conductivity or PEC boundaries.\nWarnings:\n - Volumes: ['air', 'core']"
]
}
],
"source": [
"results = sim.run()"
]
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

This cell fails with a ValueError during mesh validation, specifically complaining about the absence of conductor surfaces. This seems to indicate that the validation logic in PalaceSimMixin.validate_mesh() doesn't correctly handle purely dielectric simulations, which is common for photonic components like this directional coupler.

Since this notebook serves as an example for the new curved-element meshing feature, it's critical that it runs successfully. The validation should likely be relaxed for simulations that only use wave ports on dielectric materials.

Copy link
Copy Markdown
Contributor

@joamatab joamatab left a comment

Choose a reason for hiding this comment

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

LGTM - curved-element meshing and Palace 3D photonics

@dot-cross dot-cross requested review from cdaunt and flaport as code owners May 25, 2026 21:24
@codecov
Copy link
Copy Markdown

codecov Bot commented May 25, 2026

Codecov Report

❌ Patch coverage is 50.94851% with 181 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.25%. Comparing base (710d2c2) to head (d3773c5).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
src/gsim/palace/base.py 24.67% 48 Missing and 10 partials ⚠️
src/gsim/viz.py 14.54% 44 Missing and 3 partials ⚠️
src/gsim/palace/mesh/geometry.py 61.76% 16 Missing and 10 partials ⚠️
src/gsim/palace/mesh/gmsh_utils.py 73.03% 13 Missing and 11 partials ⚠️
src/gsim/palace/mesh/generator.py 51.11% 18 Missing and 4 partials ⚠️
src/gsim/palace/mesh/config_generator.py 62.50% 1 Missing and 2 partials ⚠️
src/gsim/common/stack/extractor.py 83.33% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #131      +/-   ##
==========================================
+ Coverage   56.18%   56.25%   +0.06%     
==========================================
  Files          59       59              
  Lines        6966     7361     +395     
  Branches     1270     1386     +116     
==========================================
+ Hits         3914     4141     +227     
- Misses       2642     2764     +122     
- Partials      410      456      +46     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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

Successfully merging this pull request may close these issues.

3 participants