Conversation
Added a serpent interface and initial solution generation for continuous variables
| def __init__(self, input, noise=1e-6, random_state=None): | ||
| self.input = input | ||
| self.dimensions = self.parse_dimensions() # List of length D, each an array of categories | ||
| #self.dimensions = self.parse_dimensions() # List of length D, each an array of categories |
There was a problem hiding this comment.
Did you forget to un-comment or is this purposeful?
| self.binary_dims = [self.calculate_binary_length(len(dim_cats)) for dim_cats in self.dimensions] | ||
| self.total_features = sum(self.binary_dims) # Total number of binary features | ||
| # Precompute the total number of binary features for LP problem | ||
| if input.calculation_type in ['single_cycle','eq_cycle']: |
There was a problem hiding this comment.
should lattice opts be added here?
| #Encode population in binary and scale in preparation to fit data | ||
| encoded_population = self.binary_encode_population(self.population) | ||
| scaled_population = self.scaler_x.fit_transform(encoded_population) | ||
| if self.input.calculation_type in ['single_cycle','eq_cycle']: |
There was a problem hiding this comment.
please change file name to include version number
| core_parameters = [self.input.nrow, self.input.ncol, self.input.num_assemblies, | ||
| self.input.symmetry, self.input.calculation_type] | ||
| if chromosome: | ||
| if chromosome is not None: |
|
|
||
| ## Initialize beginning population | ||
| self.population.current = [] | ||
| for i in range(self.population.size): |
There was a problem hiding this comment.
why was this added? the same function is performed 3 lines down?
| ## Execute and parse objective/constraint values | ||
| self.population.current = pool.starmap(self.eval_func, zip(self.population.current, repeat(self.input))) | ||
| if 'cost_fuelcycle' in self.input.objectives.keys(): | ||
| if 'cost_fuelcycle' in self.input.objectives.keys() and self.input.code_interface not in ['serpent','function']: |
There was a problem hiding this comment.
I don;t think that the conditions after the and are necessary?
| if "normalized_continuous_range" in value: | ||
| chromosome.append(random.uniform(0.0, 1.0)) | ||
| elif "normalized_discrete_range" in value: | ||
| chromosome.append(random.choice(value["normalized_discrete_range"])) |
There was a problem hiding this comment.
I don't think "narmalized" is required but I'll accept it
JakeMikouchi
left a comment
There was a problem hiding this comment.
some minor changes and questions. My biggest question is why does BO not use the contraceptive check. You will have to tell me why when we get the chance
| logger.debug(f"Changing to new working directory: {indv_dir}") | ||
| os.chdir(indv_dir) | ||
| #Create subdirectories for base, DTC, and depletion runs | ||
| if not base_dir.exists(): |
There was a problem hiding this comment.
I suggest you put all of the file/folder building in its own function to clean up evaluate()
There was a problem hiding this comment.
Same applies for the cleaning of the files after the calculation. One function for creating the folders/files, one function to delete files.
| dep_process = subprocess.Popen(dep_cmd) | ||
|
|
||
| #Unrealistically bad results to return if code fails to drive optimization away from this region | ||
| fail_results = {'doppler_temperature_coefficient': 5, 'cycle_length': 0.01, 'cost_fuelcycle': 1000000000000,'total_mass': 1000000000,'fdeltah': 8,'max_doserate':50000} |
There was a problem hiding this comment.
results parsing and handling should be put in serpent_get_results
|
|
||
| else: | ||
| peaking_results = [] #!TODO: Add parsing logic for going through detector results | ||
| peaking_results = peaking_results[peaking_results != 0] |
| #Scale the candidates to fit the data | ||
| scaled_candidates = self.scaler_x.transform(candidates) | ||
| else: | ||
| scaled_candidates = candidates |
There was a problem hiding this comment.
Do you need to put this if statement in every function ? Seems a bit redundant. Can you put it somewhere else where this is coded only once ? I think a good coding principle is to use decorators for such situations (i.e. when the same if else statement is present in multiple functions). If you are not familiar with decorators, check them and see if you can use them for this.
| def acquisition_function(x): | ||
| binary_vec = self.real_vector_to_binary(x) | ||
| if self.input.calculation_type in ['single_cycle','eq_cycle']: | ||
| vec = self.real_vector_to_binary(x) |
There was a problem hiding this comment.
might be good to use a more descriptive name for a variable than "vec"
|
|
||
| #Create LWR core parameters for use in checking constraints | ||
| core_parameters = [self.input.nrow, self.input.ncol, self.input.num_assemblies, self.input.symmetry] | ||
| #core_parameters = [self.input.nrow, self.input.ncol, self.input.num_assemblies, self.input.symmetry] |
There was a problem hiding this comment.
If this line is not needed just delete it
| if optools.Gene_Validity_check.abortive_check(self.input, gene_list, self.input.genome, core_parameters, candidate): | ||
| candidates.append(candidate) | ||
| return candidates No newline at end of file | ||
| #if optools.Gene_Validity_check.abortive_check(self.input, gene_list, self.input.genome, core_parameters, candidate): |
There was a problem hiding this comment.
same as previous comment.
| new_item = item | ||
| if not isinstance(new_item, bool): | ||
| raise ValueError("'apply' flag for input template must be true or false") | ||
| elif new_key == 'depletion_steps': |
There was a problem hiding this comment.
Do you need to parametrize all these inputs ? Overall, it is good to parametrize through the input, parameters such as the number of mpis, neutron histories etc. However, very specific aspects of the model, like the detectors or the depletion step might be better left for the input template. The reason is that if this input is too specific for a case only it cannot be practically re-used in other Serpent models.
|
|
||
| P_feed = 31.35 # USD/lb_U3O8 | ||
| P_conv = 5.00 # USD/kg_U3O8 | ||
| P_enr = 105 # USD/SWU |
There was a problem hiding this comment.
Because we will start having multiple cost functions, we can first maybe change the name of the file because it is referencing LWR. Also, it might be better to make the general parameters as global or maybe make this as a class. So we should define some default values for these parameters and then modified if needed for a specific case.
| def get_precision(x): | ||
| """Return the number of decimal places needed to represent x exactly. Used for rounding in case of floating point errors""" | ||
| d = Decimal(str(x)) | ||
| return -d.as_tuple().exponent |
There was a problem hiding this comment.
This needs to be defined within the function ? I do not like nested functions if it is not necessary
gkdelipei
left a comment
There was a problem hiding this comment.
Check our comments. We can discuss them in a separate meeting.
I updated the optimizer.py file to handle the new serpent code interface, as well as the addition of the interface itself. The bayesian optimization file can now handle continuous variables as well. Serpent interface can handle base case, doppler temperature coefficient with automatic temperature updates, and automatic depletion runs. If any one of multiple required runs fails, it will consider the evaluation as a whole to have failed and result in unrealistic results to be thrown to drive the optimization away.