Open
Conversation
Adds a variant on both SimulationNormalisation and ConventionNormalisation that provides very similar behaviour, but without the option to redefine units after creation. Also avoids splitting funcitonality between two classes that each depend on each other. Started work on a class to bundle together LocalGeometry, LocalSpecies, Numerics, and a Normalisation. The aim is for this to be an immutable representation of a single simulation, and modifications to it will generate a new instance.
Creates new norms when converting from GS2 to GENE etc. Only implemented to start from GS2, and only as a demonstration
- Uses raw variables in place of dicts for species params. - Has a separate function call per lref, bref, mref, etc - Added _mutate function that can convert convention, geometry, and species all at once. Added nicer functions with reduced argument lists. - LocalGKSimulation __init__ does simple initialisation, classmethod new() builds from sensible inputs. This is because other methods on the class need to construct new instances, and calling self.__class__.__new__(self.__class__) feels wrong. - Simplified logic in some places. Probably made it worse in others.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a demonstration of a possible solution to Issues #326 and #337. I don't expect this to ever be merged!
The primary goal here is to develop a
Normalisationclass that both merges the functionality ofSimulationNormalisationandConventionNormalisation, and forbids the redefinition of units after creation. This would avoid the need to regularly rebuild the pint cache, which is causing significant performance issues, and would guarantee that physical units (e.g.lref_minor_radius_myrun0001) would always remain valid after definition.There are two new classes that facilitate this:
ConstNormalisation: The newNormalisationsubclass (this would just be renamedNormalisationin the final version).LocalGKSimulation: A class that wraps up aConstNormalisation,LocalGeometry,LocalSpecies, andNumerics. It permits the user to generate new instances of itself with new units conventions, geometries, species, or numerics. For example:In this case,
new_simulationwill contain a brand newConstNormalisation,LocalGeometry,LocalSpecies, andNumerics. Editing any of these new instances will not overwrite details of the originals. More complex transformations can be performed by chaining function calls:Crucially, only
LocalGKSimulationis responsible for generating new normalisations. Currently, physical units are set up as side effects of several functions throughout the code, and this makes it difficult to track how they evolve through the program.I'm hoping to move towards a model in which
LocalGeometryandLocalSpeciescan be created using either simulation units or physical units via standard constructors, and, besides each having anormalise/with_unitsfunction that returns new instances, they won't have to worry about managingnormsthemselves. For now, I've had to make a few temporary edits to these classes just to get things working.In the final version,
LocalGKSimulationwould be the object returned byGKInputreaders, and the object passed in to write new input files. I'd like to seePyroact as a manager ofLocalGKSimulationinstances, which I think would be a lot simpler than its current context-switching design.LocalGKSimulationwould also be an input toGKOutput, as its contain all the needed units, geometry details, etc. It would also avoid the need to create deepcopies throughout the project, asPyroScancould make newLocalGKSimulationinstances instead.Examples of how to use this new interface are in the added tests.
A proper implementation would need to be split into several milestones. Issue #336 would be a good place to start. There are also several other issues I ran into that I'm not sure how to handle:
I made steps towards the removal of units such as
tref_deuterium,mref_hydrogen, etc, and opted instead for justtref_electronandtref_ion, as although deuterium isn't always present, we can always guarantee the presence of at least one negative species and one positive species. However, I'm aware that this won't cover many cases. For example, what if masses are defined relative to deuterium, buttrefandnrefare defined relative to a merged species representing a deuterium/tritium blend? I'll need a more complex solution than the one presented here.There's a new way to set up
SimulationNormalisationfrom aGKInput, which defines a new bespokeConventionNormalisation. This method usesset_all_referencesandadd_convention_normalisationin place ofset_lref,set_brefandset_kinetic_references, but performs very similar jobs. I'm having a hard time figuring out all the minor differences, but one of the major ones is that arbitraryvrefunits can be set up. I'm not sure how best to handle this.I'm not handling
beta/beta_refproperly.LocalSpeciesdefinesdomega_drho, which is always defined in normalised units, even if generated from aKineticsinstance. This would have to be replaced with a more generaldomega_dr, which would either have dimensions of[time]^-1 [length]^-1or[vref] [lref]^-2. Currently it always needs to be in terms of[lref]. I'm unsure if this would break things.