terrapower / armi

An open-source nuclear reactor analysis automation framework that helps design teams increase efficiency and quality
https://terrapower.github.io/armi/
Apache License 2.0
232 stars 89 forks source link

Remove references to geom files in the code #450

Open youngmit opened 3 years ago

youngmit commented 3 years ago

Back in the day, we used to specify core layout in what was called a facemap or geom file. While we retain the code that supports this input mechanism (see https://github.com/terrapower/armi/blob/master/armi/reactor/systemLayoutInput.py), it is mostly there to enable the migration performed here: https://github.com/terrapower/armi/blob/190c0caccfd36a0442118a1ae666c2a20c52a4d1/armi/reactor/blueprints/__init__.py#L524

Instead we recommend using grid blueprints instead. However, when these were completely separate input files, they became part of some major ARMI function signatures where input stuff gets passed around, usually as geom. We should try to remove as many of these as we can.

Actually searching around, I'm not seeing as many as I remember (go us!), but a glaring example is in the Case class: https://github.com/terrapower/armi/blob/190c0caccfd36a0442118a1ae666c2a20c52a4d1/armi/cases/case.py#L73

john-science commented 3 years ago

What about this Database method:

https://github.com/terrapower/armi/blob/190c0caccfd36a0442118a1ae666c2a20c52a4d1/armi/bookkeeping/db/database.py#L78

and

https://github.com/terrapower/armi/blob/190c0caccfd36a0442118a1ae666c2a20c52a4d1/armi/bookkeeping/db/database3.py#L790

Are those geomString arguments still necessary?

john-science commented 2 years ago

@drewejohnson @keckler @jakehader Would any of you notice or care if ARMI finally dropped all support for the old Geometry XML files?

First off, I'm not sure we have full, functioning support for the old XMLs any more. I assume I can nuke all references and support for the old Geometry XML files and it won't bother anyone.

Tell me if I'm wrong.

keckler commented 2 years ago

In fact, I would love it if XML support was dropped.

john-science commented 2 years ago

The following files will need to be re-written in YAML and moved into a blueprints file:

Though, maybe some/all of them will be straight deletable.

keckler commented 2 years ago

So, can you remind me the status of this?

I know we removed support for XML case settings in #612 .

But did we ever remove support for the XML geom file? I know that Jake said he would be fine with it, but did it ever actually happen? I can't find any record of that being finally deprecated.

john-science commented 2 years ago

I would love to remove geom files entirely.

There are still a couple of features blueprint files don't support that need translating over. I can't find my notes at the moment, but if memory serves, ARMI only supports ThetaRZ geometries through geomFiles, and not blueprint YAML files.

So, the last couple geomFile-only features need to be supported by blueprint YAMLs, then we need to help everyone downstream translate their inputs, THEN we can finally remove geomFiles.

john-science commented 2 years ago

Okay, I can only find the one feature left unsupported by blueprint files: ThetaRZ reactors. So, these two files:

So, we need a blueprint/YAML format to replace the old XML for ThetaRZ:

<reactor geom="ThetaRZ" symmetry="full core">
<assembly theta1="0" theta2="360.0" rad1="0" rad2="2.0" name="IC" azimuthalMesh="1" radialMesh="1"/>
<assembly theta1="0" theta2="180" rad1="2.0" rad2="3.0" name="MC" azimuthalMesh="1" radialMesh="1"/>
<assembly theta1="180" theta2="360" rad1="2.0" rad2="3.0" name="MC" azimuthalMesh="1" radialMesh="1"/>
<assembly theta1="0" theta2="360" rad1="3.0" rad2="4.0" name="OC" azimuthalMesh="1" radialMesh="1"/>
</reactor>
john-science commented 1 year ago

According to @ntouran , the place in the code I need to improve to remove these geom files is in gridBlueprints.py, where we currently read the geom XML file versions of the RZ Theta blueprints.

That will have to be improved to fully support all the little bits and pieces we use for RZ Theta XML blueprints.

john-science commented 1 year ago

Okay, we definitely support basic RZTheta geometries in YAML format.

Nick believes there are some extra pieces of information in the RZTheta geom files that isn't supported in RZTheta YAML files. So I need to find the specifics of what we don't support, and build support for it.

john-science commented 1 year ago

This is still quite important. Though the features that XML geomFiles support for RZ-Theta geometries are still unknown to me.

I believe the problem is something like: RZTheta "geomFiles" have some undocumented feature they support that isn't supported in our YAML blueprints.

But the "undocumented feature" is the problem.

Per our previous discussion: @opotowsky @albeanth

albeanth commented 1 year ago

Is there anyway we can give a deadline for folks to chime in on this? Then after the deadline we yank out the geom files and be done with it.

john-science commented 1 month ago

Okay, it has been years now, and I have not found this known RZ-Theta feature that our XML format supports that our YAML format doesn't.

I guess my new goal is to just convert any XML files we have around to YAML and see what happens.

I would like to start with this downstream of ARMI, as that is probably where the most complicated XML files are at. But I would need to find those file and users.

@sombrereau @keckler Do you guys know anyone still using these awful ARMI XML files?

keckler commented 1 month ago

I do not.